Re: [PATCH 02/16] test-lib: bring $remove_trash out of retirement

Junio C Hamano [off-list ref] writes:

Ævar Arnfjörð Bjarmason  [off-list ref] writes:

quoted

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 <redacted>
---

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.

By the way, I did wonder if the call to test_done is even needed, or
if it is even ready to be called (in other words, the machineries it
tries to finalize like junit and HARNESS_ACTIVE are even initialized
yet).  I verified that calling test_done this early is fine with
respect to the call it makes to finalize_junit_xml, though.

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.