{"thread":{"id":"56118","subject":"[PATCH] t0000: clear GIT_SKIP_TESTS before running sub-tests","startedAt":"2021-07-16T18:43:52Z","lastAt":"2021-07-16T18:43:52Z","messageCount":1,"participants":["Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"430372","messageId":"YPHTY5G9JaQFKlX5@coredump.intra.peff.net","threadId":"56118","inReplyTo":null,"subject":"[PATCH] t0000: clear GIT_SKIP_TESTS before running sub-tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-07-16T18:43:47Z","receivedAt":"2021-07-16T18:43:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"In t0000, we run several fake \"sub-test\" suites to verify the behavior\nof the test suite. But because we don't clear the parent environment\ncompletely, the sub-tests can be fooled by variables meant for the\nparent. For example:\n\n  GIT_SKIP_TESTS=t1234 ./t0000-basic.sh\n\nfails when a sub-test expects its fake t1234 to actually run. This\nparticular pattern is unlikely in practice; we're running a single\nscript, and there is no t1234 in the real test suite anyway (not yet, at\nleast). A more real-world example is:\n\n  GIT_SKIP_TESTS=t[^0]* make test\n\nto run only the t0* tests.\n\nThe fix is conceptually simple: we should clear the GIT_SKIP_TESTS\nvariable when running the sub-tests, because its contents (if any) will\nbe meant for the main test suite. This is easy to do centrally in our\nsub-test helper.\n\nBut there's a catch: some of our tests do set GIT_SKIP_TESTS\nintentionally to test the feature. We need to allow them to continue to\nset it, but clear it for all the other tests. And the sub-test helper\ncan't tell if the GIT_SKIP_TESTS it sees is from a test or not. We can\nhandle this by adding a new option to the helper to let callers specify\nthe skip list.\n\nI considered adding a more general \"--eval\" option to let callers set up\nthe env for the sub-test however they like. That would cover this case\nand possible future ones. But the quoting gets awkward for the callers\n(since we're now 2 layers deep in evals!), so I went with the simpler\nmore specific solution.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nFor reference, the eval solution would look something like this in the\ncallers:\n\n\tdiff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\n\tindex f68c27d864..bf60c8fdc8 100755\n\t--- a/t/t0000-basic.sh\n\t+++ b/t/t0000-basic.sh\n\t@@ -361,7 +361,10 @@ test_expect_success 'GIT_SKIP_TESTS several tests' '\n\t \t(\n\t \t\trun_sub_test_lib_test git-skip-tests-several \\\n\t \t\t\t\"GIT_SKIP_TESTS several tests\" \\\n\t-\t\t\t--skip=\"git.2 git.5\" <<-\\EOF &&\n\t+\t\t\t--eval=\"\n\t+\t\t\t\tGIT_SKIP_TESTS=\\\"git.2 git.5\\\"\n\t+\t\t\t\texport GIT_SKIP_TESTS\n\t+\t\t\t\" <<-\\EOF &&\n\t \t\tfor i in 1 2 3 4 5 6\n\t \t\tdo\n\t \t\t\ttest_expect_success \"passing test #$i\" \"true\"\n\n t/t0000-basic.sh | 39 +++++++++++++++++++++++++++++----------\n 1 file changed, 29 insertions(+), 10 deletions(-)\n\ndiff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\nindex 2c6e34b947..f68c27d864 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -69,6 +69,23 @@ test_expect_success 'success is reported like this' '\n _run_sub_test_lib_test_common () {\n \tneg=\"$1\" name=\"$2\" descr=\"$3\" # stdin is the body of the test code\n \tshift 3\n+\n+\t# intercept pseudo-options at the front of the argument list that we\n+\t# will not pass to child script\n+\tskip=\n+\twhile test $# -gt 0\n+\tdo\n+\t\tcase \"$1\" in\n+\t\t--skip=*)\n+\t\t\tskip=${1#--*=}\n+\t\t\tshift\n+\t\t\t;;\n+\t\t*)\n+\t\t\tbreak\n+\t\t\t;;\n+\t\tesac\n+\tdone\n+\n \tmkdir \"$name\" &&\n \t(\n \t\t# Pretend we're not running under a test harness, whether we\n@@ -91,6 +108,8 @@ _run_sub_test_lib_test_common () {\n \t\texport TEST_DIRECTORY &&\n \t\tTEST_OUTPUT_DIRECTORY=$(pwd) &&\n \t\texport TEST_OUTPUT_DIRECTORY &&\n+\t\tGIT_SKIP_TESTS=$skip &&\n+\t\texport GIT_SKIP_TESTS &&\n \t\tsane_unset GIT_TEST_FAIL_PREREQS &&\n \t\tif test -z \"$neg\"\n \t\tthen\n@@ -319,9 +338,9 @@ test_expect_success 'test --verbose-only' '\n \n test_expect_success 'GIT_SKIP_TESTS' '\n \t(\n-\t\tGIT_SKIP_TESTS=\"git.2\" && export GIT_SKIP_TESTS &&\n \t\trun_sub_test_lib_test git-skip-tests-basic \\\n-\t\t\t\"GIT_SKIP_TESTS\" <<-\\EOF &&\n+\t\t\t\"GIT_SKIP_TESTS\" \\\n+\t\t\t--skip=\"git.2\" <<-\\EOF &&\n \t\tfor i in 1 2 3\n \t\tdo\n \t\t\ttest_expect_success \"passing test #$i\" \"true\"\n@@ -340,9 +359,9 @@ test_expect_success 'GIT_SKIP_TESTS' '\n \n test_expect_success 'GIT_SKIP_TESTS several tests' '\n \t(\n-\t\tGIT_SKIP_TESTS=\"git.2 git.5\" && export GIT_SKIP_TESTS &&\n \t\trun_sub_test_lib_test git-skip-tests-several \\\n-\t\t\t\"GIT_SKIP_TESTS several tests\" <<-\\EOF &&\n+\t\t\t\"GIT_SKIP_TESTS several tests\" \\\n+\t\t\t--skip=\"git.2 git.5\" <<-\\EOF &&\n \t\tfor i in 1 2 3 4 5 6\n \t\tdo\n \t\t\ttest_expect_success \"passing test #$i\" \"true\"\n@@ -364,9 +383,9 @@ test_expect_success 'GIT_SKIP_TESTS several tests' '\n \n test_expect_success 'GIT_SKIP_TESTS sh pattern' '\n \t(\n-\t\tGIT_SKIP_TESTS=\"git.[2-5]\" && export GIT_SKIP_TESTS &&\n \t\trun_sub_test_lib_test git-skip-tests-sh-pattern \\\n-\t\t\t\"GIT_SKIP_TESTS sh pattern\" <<-\\EOF &&\n+\t\t\t\"GIT_SKIP_TESTS sh pattern\" \\\n+\t\t\t--skip=\"git.[2-5]\" <<-\\EOF &&\n \t\tfor i in 1 2 3 4 5 6\n \t\tdo\n \t\t\ttest_expect_success \"passing test #$i\" \"true\"\n@@ -388,9 +407,9 @@ test_expect_success 'GIT_SKIP_TESTS sh pattern' '\n \n test_expect_success 'GIT_SKIP_TESTS entire suite' '\n \t(\n-\t\tGIT_SKIP_TESTS=\"git\" && export GIT_SKIP_TESTS &&\n \t\trun_sub_test_lib_test git-skip-tests-entire-suite \\\n-\t\t\t\"GIT_SKIP_TESTS entire suite\" <<-\\EOF &&\n+\t\t\t\"GIT_SKIP_TESTS entire suite\" \\\n+\t\t\t--skip=\"git\" <<-\\EOF &&\n \t\tfor i in 1 2 3\n \t\tdo\n \t\t\ttest_expect_success \"passing test #$i\" \"true\"\n@@ -405,9 +424,9 @@ test_expect_success 'GIT_SKIP_TESTS entire suite' '\n \n test_expect_success 'GIT_SKIP_TESTS does not skip unmatched suite' '\n \t(\n-\t\tGIT_SKIP_TESTS=\"notgit\" && export GIT_SKIP_TESTS &&\n \t\trun_sub_test_lib_test git-skip-tests-unmatched-suite \\\n-\t\t\t\"GIT_SKIP_TESTS does not skip unmatched suite\" <<-\\EOF &&\n+\t\t\t\"GIT_SKIP_TESTS does not skip unmatched suite\" \\\n+\t\t\t--skip=\"notgit\" <<-\\EOF &&\n \t\tfor i in 1 2 3\n \t\tdo\n \t\t\ttest_expect_success \"passing test #$i\" \"true\"\n-- \n2.32.0.693.gc302eace04\n"}]}