[v2] t0021: make sure clean filter runs
diff mbox series

Message ID 20190822192240.GA4077@cat
State New
Headers show
Series
  • [v2] t0021: make sure clean filter runs
Related show

Commit Message

Thomas Gummerer Aug. 22, 2019, 7:22 p.m. UTC
In t0021.15 one of the things we are checking is that the clean filter
is run when checking out empty-branch.  The clean filter needs to be
run to make sure there are no modifications on the file system for the
test.r file, and thus it isn't dangerous to overwrite it.

However in the current test setup it is not always necessary to run
the clean filter, and thus the test sometimes fails, as debug.log
isn't written.

This happens when test.r has an older mtime than the index itself.
That mtime is also recorded as stat data for test.r in the index, and
based on the heuristic we're using for index entries, git correctly
assumes this file is up-to-date.

Usually this test succeeds because the mtime of test.r is the same as
the mtime of the index.  In this case test.r is racily clean, so git
actually checks the contents, for which the clean filter is run.

Fix the test by updating the mtime of test.r, so git is forced to
check the contents of the file, and the clean filter is run as the
test expects.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---

v2 adds the comment as suggested by Szeder.

Junio: I saw this is marked as "merged to 'next'" in the What's
cooking, so if it got merged already I'm fine with just keeping v1,
but otherwise I think adding the comment would be nice.

 t/t0021-conversion.sh | 3 +++
 1 file changed, 3 insertions(+)

Comments

Junio C Hamano Aug. 22, 2019, 8:01 p.m. UTC | #1
Thomas Gummerer <t.gummerer@gmail.com> writes:

> Fix the test by updating the mtime of test.r, so git is forced to
> check the contents of the file, and the clean filter is run as the
> test expects.
>
> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
> ---
>
> v2 adds the comment as suggested by Szeder.
>
> Junio: I saw this is marked as "merged to 'next'" in the What's
> cooking, so if it got merged already I'm fine with just keeping v1,
> but otherwise I think adding the comment would be nice.

I think it was marked with "Will merge to ...".  Replaced.

Thanks.
SZEDER Gábor Aug. 23, 2019, 8:34 a.m. UTC | #2
On Thu, Aug 22, 2019 at 08:22:40PM +0100, Thomas Gummerer wrote:
> v2 adds the comment as suggested by Szeder.

> +		# Make sure that the file appears dirty, so checkout below has to
> +		# run the configured filter.

Yeah, but that comment only really applied when setting both
timestamps.  With a simple 'touch' it's more like

  # Make sure that the file appears dirty or is at least racily clean,
  # so ...

So the next reader will know that right away, that the author didn't
overlook racyness issue.

> +		touch test.r &&
>  		filter_git checkout --quiet --no-progress empty-branch &&
>  		cat >expected.log <<-EOF &&
>  			START
> -- 
> 2.23.0.rc2.194.ge5444969c9
>

Patch
diff mbox series

diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index e10f5f787f..c954c709ad 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -390,6 +390,9 @@  test_expect_success PERL 'required process filter should filter data' '
 		EOF
 		test_cmp_exclude_clean expected.log debug.log &&
 
+		# Make sure that the file appears dirty, so checkout below has to
+		# run the configured filter.
+		touch test.r &&
 		filter_git checkout --quiet --no-progress empty-branch &&
 		cat >expected.log <<-EOF &&
 			START