On Wed, Mar 17, 2021 at 01:54:25PM -0400, Jeff King wrote: > -test_expect_success !PTHREADS 'pack-objects --threads=N or pack.threads=N warns when no pthreads' ' > +test_expect_success !PTHREADS,IGNORE_FAIL_PREREQS \ > + 'pack-objects --threads=N or pack.threads=N warns when no pthreads' ' > git pack-objects --threads=2 --stdout --all </dev/null >/dev/null 2>err && > grep ^warning: err >warnings && > test_line_count = 1 warnings && > > but I think this points to a failing of the FAIL_PREREQS mode. It is > generally OK to say "skip this test by pretending you do not have a > prereq satisfied" (and that is the point: to see if skipping a test > confuses later tests). But given a negated prereq here, it is not OK to > say "run this test that we usually wouldn't", because it is almost > certainly going to be mismatched with the actual build. > > So I think the FAIL_PREREQS mode should probably be treating negated > prereqs differently (and always pretending that yes, we have them). > > I hadn't investigated the t7810 case yet, but looking at it now, it > seems to be the exact same thing. It looks like the problem is indeed somewhat widespread, and there is a magic prereq already to skip such tests. I do still think that this is a fundamental failing of the FAIL_PREREQS mode, but it probably makes sense to annotate these tests in the meantime (I don't plan on looking further into it myself). Another rough edge I noticed: if you set GIT_TEST_HTTPD or GIT_TEST_GIT_DAEMON to "yes" in your config.mak, these play quite badly with GIT_TEST_FAIL_PREREQS. We think NOT_ROOT is not satisfied, so refuse to start httpd, and then complain that the setup fails (and the point of "yes" for those values is to loudly complain when setup fails, rather than quietly skipping the tests). -- >8 -- Subject: [PATCH] t: annotate !PTHREADS tests with !FAIL_PREREQS Some tests in t5300 and t7810 expect us to complain about a "--threads" argument when Git is compiled without pthread support. Running these under GIT_TEST_FAIL_PREREQS produces a confusing failure: we pretend to the tests that there is no pthread support, so they expect the warning, but of course the actual build is perfectly happy to respect the --threads argument. We never noticed before the recent a926c4b904 (tests: remove most uses of C_LOCALE_OUTPUT, 2021-02-11), because the tests also were marked as requiring the C_LOCALE_OUTPUT prerequisite. Which means they'd never have run in FAIL_PREREQS mode, since it would always pretend that the locale prereq was not satisfied. These tests can't possibly work in this mode; it is a mismatch between what the tests expect and what the build was told to do. So let's just mark them to be skipped, using the special prereq introduced by dfe1a17df9 (tests: add a special setup where prerequisites fail, 2019-05-13). Reported-by: Son Luong Ngoc <sluongng@xxxxxxxxx> Signed-off-by: Jeff King <peff@xxxxxxxx> --- t/t5300-pack-object.sh | 6 ++++-- t/t7810-grep.sh | 3 ++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh index d586fdc7a9..e830a37a38 100755 --- a/t/t5300-pack-object.sh +++ b/t/t5300-pack-object.sh @@ -427,7 +427,8 @@ test_expect_success 'index-pack --strict <pack> works in non-repo' ' test_path_is_file foo.idx ' -test_expect_success !PTHREADS 'index-pack --threads=N or pack.threads=N warns when no pthreads' ' +test_expect_success !PTHREADS,!FAIL_PREREQS \ + 'index-pack --threads=N or pack.threads=N warns when no pthreads' ' test_must_fail git index-pack --threads=2 2>err && grep ^warning: err >warnings && test_line_count = 1 warnings && @@ -445,7 +446,8 @@ test_expect_success !PTHREADS 'index-pack --threads=N or pack.threads=N warns wh grep -F "no threads support, ignoring pack.threads" err ' -test_expect_success !PTHREADS 'pack-objects --threads=N or pack.threads=N warns when no pthreads' ' +test_expect_success !PTHREADS,!FAIL_PREREQS \ + 'pack-objects --threads=N or pack.threads=N warns when no pthreads' ' git pack-objects --threads=2 --stdout --all </dev/null >/dev/null 2>err && grep ^warning: err >warnings && test_line_count = 1 warnings && diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh index edfaa9a6d1..5830733f3d 100755 --- a/t/t7810-grep.sh +++ b/t/t7810-grep.sh @@ -969,7 +969,8 @@ do " done -test_expect_success !PTHREADS 'grep --threads=N or pack.threads=N warns when no pthreads' ' +test_expect_success !PTHREADS,!FAIL_PREREQS \ + 'grep --threads=N or pack.threads=N warns when no pthreads' ' git grep --threads=2 Hello hello_world 2>err && grep ^warning: err >warnings && test_line_count = 1 warnings && -- 2.31.0.559.g509d4a088b
