Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
> There's no point in creating a repository or directory only to decide
> right afterwards that we're skipping all the tests.
>
> So let's partially revert 06478dab4c (test-lib: retire $remove_trash
> variable, 2017-04-23) and move the decision about whether to skip all
> tests earlier.
>
> I tested this with --debug, see 4d0912a206 (test-lib.sh: do not barf
> under --debug at the end of the test, 2017-04-24) for a bug we don't
> want to re-introduce.
>
> While I'm at it let's move the HOME assignment to just before
> test_create_repo, it could be lower, but it seems better to set it
> before calling anything in test-lib-functions.sh
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
> ---
I am not clear what the reasoning behind this change is. Is it
correct to say that the idea flows like the following?
0. If we do the $GIT_SKIP_TESTS check early, specifically, before
we create the trash directory to run the tests in, we do not
have to create and then remove it.
1. If we naïvely do the above, test_done called in the "ah, the
entirety of test is skipped" case will try to see if the trash
directory exists and complain, so we need to add more code in
that test_done codepath to avoid complaints.
2. As a part of that "more code", we mark the fact that we created
a new repository with $remove_trash variable.
Assuming that the above flow of thought is what is behind this
patch, I think the patch is mostly sensible.
remove_trash needs to be cleared at the beginning, perhaps where
store_arg_to and opt_required_arg are cleared, though, to protect
us from a stray environment variable.
Thanks.
