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

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 28, 2022, 22:19 UTC
Message-ID: <xmqqr17my6pq.fsf@gitster.g>
In-Reply-To: <a1806c56333ee96f9cda8c77ae56517181b691c0.1646041237.git.gitgitgadget@gmail.com>

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

Show 13 quoted lines

> From: Tao Klerks <tao@klerks.biz>
>
> The untracked cache test uses an avoid_racy function to deal with
> an mtime-resolution challenge in testing: If an untracked cache
> entry's mtime falls in the same second as the mtime of the index
> the untracked cache was stored in, then it cannot be trusted.
>
> Explicitly delaying tests is a simple effective strategy to
> avoid these issues, but should be avoided where possible.
>
> Switch from a delay-based strategy to instead backdating
> all file changes using test-tool chmtime, where that is an
> option, to shave 9 seconds off the test run time.

OK.

> Don't update test cases that delay for other reasons, for now at
> least (4 seconds).

Sad but one step at a time.

> +chmmtime_worktree_root () {

Not "chmtime_worktree_root"?  Is doubled-m intended?

Show 5 quoted lines

> +	# chmtime doesnt handle relative paths on windows, so need
> +	# to "hardcode" a reference to the worktree folder name.
> +	cd .. &&
> +	test-tool chmtime $1 worktree &&
> +	cd worktree

An unsuspecting caller will be left in a directory it didn't expect to when "test-tool chmtime" fails, which is not nice.

	(
		cd .. &&
		test-tool chmtime "$1" worktree
	)

at least until the tool learns to do

	test-tool chmtime "$1" ../worktree

or

	test-tool -C.. chmtime "$1" worktree

Oh, isn't the last one already available?

Show 11 quoted lines

> +}
> +
>  avoid_racy() {
>  	sleep 1
>  }
> @@ -90,6 +98,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 &&

Not a huge deal, but it would have been very nice if "test-tool chmtime" had a "-R" option to recurse into directories.