Re: [PATCH v3 08/17] t4216: test changed path filters with high bit paths

From: Taylor Blau <me@ttaylorr.com>
Date: Oct 18, 2023, 17:41 UTC
Message-ID: <ZTAYvKBeqUyU7o4W@nand.local>
In-Reply-To: <ZS5JmQib3onjirC6@tanuki>

On Tue, Oct 17, 2023 at 10:45:13AM +0200, Patrick Steinhardt wrote:

Show 25 quoted lines

> > +test_expect_success 'setup check value of version 1 changed-path' '
> > +	(
> > +		cd highbit1 &&
> > +		echo "52a9" >expect &&
> > +		get_first_changed_path_filter >actual &&
> > +		test_cmp expect actual
> > +	)
> > +'
> > +
> > +# expect will not match actual if char is unsigned by default. Write the test
> > +# in this way, so that a user running this test script can still see if the two
> > +# files match. (It will appear as an ordinary success if they match, and a skip
> > +# if not.)
> > +if test_cmp highbit1/expect highbit1/actual
> > +then
> > +	test_set_prereq SIGNED_CHAR_BY_DEFAULT
> > +fi
> > +test_expect_success SIGNED_CHAR_BY_DEFAULT 'check value of version 1 changed-path' '
> > +	# Only the prereq matters for this test.
> > +	true
> > +'
>
> Doesn't this mean that the preceding test where we `test_cmp expect
> actual` can fail on some platforms depending on the signedness of
> `char`?

Great catch, I am surprised this slipped by in earlier rounds. This should do the trick, since we don't actually care about conditioning that test's passing on test_cmp coming up clean. We check that in the if statement you pointed out here, so:

--- 8< ---

diff --git a/t/t4216-log-bloom.sh b/t/t4216-log-bloom.sh
index 114672e904..400dce2193 100755
--- a/t/t4216-log-bloom.sh
+++ b/t/t4216-log-bloom.sh
@@ -502,8 +502,7 @@ test_expect_success 'setup check value of version 1 changed-path' '
 	(
 		cd highbit1 &&
 		echo "52a9" >expect &&
-		get_first_changed_path_filter >actual &&
-		test_cmp expect actual
+		get_first_changed_path_filter >actual
 	)
 '
--- >8 ---

Thanks,
Taylor