Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes: > On Fri, Jan 17, 2025 at 8:59 AM shejialuo <shejialuo@xxxxxxxxx> wrote: >> On Tue, Jan 07, 2025 at 08:33:56AM -0800, Karthik Nayak wrote: >> > shejialuo <shejialuo@xxxxxxxxx> writes: >> > > +test_expect_success SYMLINKS 'the filetype of packed-refs should be checked' ' >> > > + test_when_finished "rm -rf repo" && >> > > + git init repo && >> > > + cd repo && >> > >> > This should be in a subshell, so that at the end we can actually remove >> > the repo. This seems to be applicable to most of the other tests in this >> > file too. Perhaps, we should clean it up as a precursor commit to this >> > series? >> >> I have searched the usage of "test_when_finished", and I don't know why >> we need to use subshell. Could you please explain this further here. > > Karthik may have been thinking about operating systems, such as > Microsoft Windows, which won't allow a directory to be deleted if that > directory is in use. In this case, because the test cd's into "repo" > and never cd's elsewhere, the directory is still in use when > test_when_finished() tries to delete "repo". > I didn't know this either. I was mostly talking about what you mentioned below. > However, there is an even more important reason to use a subshell, and > that is because a subshell ensures that the current working directory > is effectively restored to the path which was current before the cd > command. This is important since it guarantees that subsequent tests > will be run in the correct directory even if the preceding test bombed > out part way through. For example: > > test_expect_success 'foo' ' > git init repo && > cd repo && > ...some more commands... && > cd .. > ' > > If one of the commands in "...some more commands..." fails, then the > `cd ..` will never be reached, and the current working directory will > remain "repo" rather than reverting to the path prior to the cd > command. Thus, any tests which follow this one in the script will end > up running in the wrong directory. The proper way to protect against > this is: > > test_expect_success 'foo' ' > git init repo && > ( > cd repo && > ...some more commands... > ) > ' > > Exiting the subshell will correctly restore the current working > directory to the original path _regardless_ of whether the test > succeeds or fails somewhere in "...some more commands...". Using a > subshell also means that you don't have to manually restore the > working directory via `cd ..` or similar. This is was a super nice explanation compared to my single sentence. Thanks!
Attachment:
signature.asc
Description: PGP signature
