Jeff King <peff@xxxxxxxx> writes: >> 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). The README file in t/ directory claims that this "is useful for discovering issues with the tests where say a later test implicitly depends on an optional earlier test." but apparently it does not work well with these negated prerequisites. Its implementation probably should force a safe bypass of the whole test_have_prereq() etc. done in test_skip by hooking into test_verify_prereq and overwrite any non-empty test_prereq with a single hardcoded PRETEND_FAIL_PREREQ prerequisite that is never satisfied, or something. > 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). ... and I think this would also be gone, as the NOT_ROOT test is done with test_have_prereq that we wouldn't be mucking with if we limit the FAIL_PREREQS only to tweak the test_expect_* prereqs. In short, the biggest mistake in the current FAIL_PREREQS design is to hook into test_have_prereq while the stated objective only needs to futz with the prerequisite given to the test_expect_* functions, I would think. > -- >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 &&
