Re: [PATCH v2 2/2] t7063: mtime-mangling instead of delays in untracked cache testing

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

"Tao Klerks via GitGitGadget" <gitgitgadget@gmail.com> writes:

> +chmtime_worktree_root () {
> +	# chmtime doesnt handle relative paths on windows, so need
> +	# to "hardcode" a reference to the worktree folder name.
> +	test-tool -C .. chmtime $1 worktree
> +}
> +

Enclose $1 in a pair of double-quotes to help readers.  They do not
have to wonder if the caller is interested in (or has to worry
about) triggering word splitting at $IFS if you did so.

>  avoid_racy() {
>  	sleep 1
>  }
> @@ -90,6 +96,9 @@ test_expect_success 'setup' '
>  	cd worktree &&
>  	mkdir done dtwo dthree &&
>  	touch one two three done/one dtwo/two dthree/three &&
> +	test-tool chmtime =-300 one two three done/one dtwo/two dthree/three &&
> +	test-tool chmtime =-300 done dtwo dthree &&
> +	chmtime_worktree_root =-300 &&

I am wondering if it is better to spelling it out like this:

	test-tool -C.. chmtime =-300 worktree &&

instead of hiding the fact that "../worktree" is being touched
behind a one-line helper.  Being able to explicitly write "worktree"
in the context that this particular code path uses the "worktree"
directory is a big plus, but at the same time, bypassing the helper
makes it unclear why we just don't chmtime "../worktree", and will
strongly tempt future developers into breaking it, so, I dunno.

What's the reason why utime() works only on a path in the current
directory and cannot take "../worktree" again?  If we cannot solve
that, I guess an extra helper with a big comment, like we see in
this patch, would be the least bad solution.

Thanks.