Re: [PATCH v2 1/8] t0602: use subshell to ensure working directory unchanged

[Junio C Hamano <gitster@pobox.com>]

shejialuo <shejialuo@gmail.com> writes:

> For every test, we would execute the command "cd repo" in the first but
> we never execute the command "cd .." to restore the working directory.
> However, it's either not a good idea use above way. Because if any test
> fails between "cd repo" and "cd ..", the "cd .." will never be reached.
> And we cannot correctly restore the working directory.
>
> Let's use subshell to ensure that the current working directory could be
> restored to the correct path.
>
> Mentored-by: Patrick Steinhardt <ps@pks.im>
> Mentored-by: Karthik Nayak <karthik.188@gmail.com>
> Signed-off-by: shejialuo <shejialuo@gmail.com>
> ---
>  t/t0602-reffiles-fsck.sh | 967 ++++++++++++++++++++-------------------
>  1 file changed, 494 insertions(+), 473 deletions(-)

Note for bystanders who may be interested in helping to ensure
correctness of this step.

The patch meant for the machines we see here is unreadable for
humans [*], but the result of applying it and then running

    $ git show -wW t/

gives me a very clear "from here to there, the entire thing now has
a pair of () around it" pattern.  If you look at the clean-up step
each test piece defines with test_when_finished at the front, and
comparing it with the directory name the test repository "git init"
in each test piece creates and "cd" goes into, it is fairly easy to
see that the patch is doing the right thing without doing anything
unwanted.

All the here-doc in the test are now indented one level deeper, but
you can check that they use "<<-EOF" to be oblivious to the leading
tabs, making this conversion a safe one.

One thing that is hard to validate by code inspection alone is

 - This change will change the commit timestamps of the commits
   created by "test_commit" helper function, now that they are run
   in subshells to get their internal clock reset in each test
   piece.

But if the tests rely on the exact commit object names, running the
resulting script just once would be sufficient to notice.

Overall, very nicely done.

Queued.  Thanks.


[Footnote]

 * No, I do not mean to say that you should spend time trying to
   make the message readable by humans in a case like this.  A patch
   that can be mechanically processed and leave the byte sequence
   you intended to give the recipients is exactly what we want.