+ cc Jens as he authored both t6041 as well as t3513 in
the series leading to ad25da009e2a3730 (2014-07-21,
Merge branch 'jl/submodule-tests')
On Mon, May 9, 2016 at 11:46 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Stefan Beller <sbeller@xxxxxxxxxx> writes:
>
>> diff --git a/t/t6041-bisect-submodule.sh b/t/t6041-bisect-submodule.sh
>> index c6b7aa6..62b8a2e 100755
>> --- a/t/t6041-bisect-submodule.sh
>> +++ b/t/t6041-bisect-submodule.sh
>> @@ -8,7 +8,7 @@ test_description='bisect can handle submodules'
>> git_bisect () {
>> git status -su >expect &&
>> ls -1pR * >>expect &&
>> - tar czf "$TRASH_DIRECTORY/tmp.tgz" * &&
>> + tar cf "$TRASH_DIRECTORY/tmp.tar" * &&
>> GOOD=$(git rev-parse --verify HEAD) &&
>> git checkout "$1" &&
>> echo "foo" >bar &&
>> @@ -20,7 +20,7 @@ git_bisect () {
>> git bisect start &&
>> git bisect good $GOOD &&
>> rm -rf * &&
>> - tar xzf "$TRASH_DIRECTORY/tmp.tgz" &&
>> + tar xf "$TRASH_DIRECTORY/tmp.tar" &&
>> git status -su >actual &&
>> ls -1pR * >>actual &&
>> test_cmp expect actual &&
>
> While I am fine with taking this (and the other) change, which are
> the only sensible response to these bug reports, this makes me
> wonder two things and a half.
>
> * Why do we even run "tar", especially without having a
> test_when_finished clean-up? Can't there be better ways to test
> this and the other one without creating a copy of everything in
> the test directory?
I think some of the submodule testing is a test machinery on its own.
Any submodule test that is not in t74* follows the pattern to
provide a short function for its testing and then call test_submodule_switch
with some long option, e.g.
KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1
I haven't attempted to look into these tests yet as they seem to test
fundamentals ("submodules as files" in other commands), whereas
I am more interested in some new features currently.
>
> * Are we sure the trash directory contents is kept manageable size?
The testing in lib-submodule-update.sh do not seem to take care of
these tarballs. And the small testing scripts do not either, but there
we could inject a
test_when_finished "rm ..."
snippet IIUC.
> I am primarily worried about this "we are not sure what we will
> be clobbering, so let's blindly tar up everything and restore it
> later" pattern spreading and people mistakenly use it in tests
> that simulate our behaviour on a huge blob with a sparse but
> gigantic file.
>
> * Is an addition of 'test_snapshot $tarball *' and 'test_restore
> $tarball' pair too much unnecessary refactoring to cater to only
> two users of this "let's tar up everything" pattern, given that
> we want to _discourage_ use of this pattern in the longer term?
As said before, I did not yet dive into these test areas myself.
>From a birds eye, there was not a lot of discussion around that series
(as compared to submodule groups for example). Maybe I am missing
prior or later series though.
http://thread.gmane.org/gmane.comp.version-control.git/251682
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html