{"push_id":615026117,"size":45,"distinct_size":3,"ref":"refs/heads/jk/preapprove-helper-credentials","head":"5993837296ae5dcd38c9977031a91caf40c1b0b5","before":"88c596232138616de6e705593ff01b222e620fb5","commits":[{"sha":"bb79af9d09709d6d93a01d1494d0ca93ff6d4b99","author":{"email":"696e3fbcf235d40b6ea1ebd61c87cbea79d444e7@peff.net","name":"Jeff King"},"message":"t/test-lib: introduce --chain-lint option\n\nIt's easy to miss an \"&&\"-chain in a test script, like:\n\n  test_expect_success 'check something important' '\n\tcmd1 &&\n\tcmd2\n\tcmd3\n  '\n\nThe test harness will notice if cmd3 fails, but a failure of\ncmd1 or cmd2 will go unnoticed, as their exit status is lost\nafter cmd3 runs.\n\nThe toy example above is easy to spot because the \"cmds\" are\nall the same length, but real code is much more complicated.\nIt's also difficult to detect these situations by statically\nanalyzing the shell code with regexps (like the\ncheck-non-portable-shell script does); there's too much\ncontext required to know whether a &&-chain is appropriate\non a given line or not.\n\nThis patch instead lets the shell check each test by\nsticking a command with a specific and unusual return code\nat the top of each test, like:\n\n  (exit 117) &&\n  cmd1 &&\n  cmd2\n  cmd3\n\nIn a well-formed test, the non-zero exit from the first\ncommand prevents any of the rest from being run, and the\ntest's exit code is 117. In a bad test (like the one above),\nthe 117 is lost, and cmd3 is run.\n\nWhen we encounter a failure of this check, we abort the test\nscript entirely. For one thing, we have no clue which subset\nof the commands in the test snippet were actually run.\nRunning further tests would be pointless, because we're now\nin an unknown state. And two, this is not a \"test failure\"\nin the traditional sense. The test script is buggy, not the\ncode it is testing. We should be able to fix these problems\nin the script once, and not have them come back later as a\nregression in git's code.\n\nAfter checking a test snippet for --chain-lint, we do still\nrun the test itself.  We could actually have a pure-lint\nmode which just checks each test, but there are a few\nreasons not to. One, because the tests are executing\narbitrary code, which could impact the later environment\n(e.g., that could impact which set of tests we run at all).\nAnd two, because a pure-lint mode would still be expensive\nto run, because a significant amount of code runs outside of\nthe test_expect_* blocks.  Instead, this option is designed\nto be used as part of a normal test suite run, where it adds\nvery little overhead.\n\nTurning on this option detects quite a few problems in\nexisting tests, which will be fixed in subsequent patches.\nHowever, there are a number of places it cannot reach:\n\n - it cannot find a failure to break out of loops on error,\n   like:\n\n     cmd1 &&\n     for i in a b c; do\n\t     cmd2 $i\n     done &&\n     cmd3\n\n   which will not notice failures of \"cmd2 a\" or \"cmd b\"\n\n - it cannot find a missing &&-chain inside a block or\n   subfunction, like:\n\n     foo () {\n\t     cmd1\n\t     cmd2\n     }\n\n     foo &&\n     bar\n\n   which will not notice a failure of cmd1.\n\n - it only checks tests that you run; every platform will\n   have some tests skipped due to missing prequisites,\n   so it's impossible to say from one run that the test\n   suite is free of broken &&-chains. However, all tests get\n   run by _somebody_, so eventually we will notice problems.\n\n - it does not operate on test_when_finished or prerequisite\n   blocks. It could, but these tends to be much shorter and\n   less of a problem, so I punted on them in this patch.\n\nThis patch was inspired by an earlier patch by Jonathan\nNieder:\n\n  http://article.gmane.org/gmane.comp.version-control.git/235913\n\nThis implementation and all bugs are mine.\n\nSigned-off-by: Jeff King <peff@peff.net>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>","distinct":false,"url":"https://api.github.com/repos/peff/git/commits/bb79af9d09709d6d93a01d1494d0ca93ff6d4b99"},{"sha":"8fb268720e8565885039e7491f4628974e2d66a2","author":{"email":"696e3fbcf235d40b6ea1ebd61c87cbea79d444e7@peff.net","name":"Jeff King"},"message":"t: fix severe &&-chain breakage\n\nThese are tests which are missing a link in their &&-chain,\nin a location which causes a significant portion of the test\nto be missed (e.g., the test effectively does nothing, or\nconsists of a long string of actions and output comparisons,\nand we throw away the exit code of at least one part of the\nstring).\n\nSigned-off-by: Jeff King <peff@peff.net>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>","distinct":false,"url":"https://api.github.com/repos/peff/git/commits/8fb268720e8565885039e7491f4628974e2d66a2"},{"sha":"60687de5ba3eb90f8332fb1bcec73a2cb782c95c","author":{"email":"696e3fbcf235d40b6ea1ebd61c87cbea79d444e7@peff.net","name":"Jeff King"},"message":"t: fix moderate &&-chain breakage\n\nThese are tests which are missing a link in their &&-chain,\nbut in a way that probably does not effect the outcome of\nthe test. Most of these are of the form:\n\n  some_cmd >actual\n  test_cmp expect actual\n\nThe main point of the test is to verify the output, and a\nfailure in some_cmd would probably be noticed by bogus\noutput. But it is good for the tests to also confirm that\n\"some_cmd\" does not die unexpectedly after producing its\noutput.\n\nSigned-off-by: Jeff King <peff@peff.net>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>","distinct":false,"url":"https://api.github.com/repos/peff/git/commits/60687de5ba3eb90f8332fb1bcec73a2cb782c95c"},{"sha":"99094a7ad484085b9f6849a8ebe1e23467c5bc14","author":{"email":"696e3fbcf235d40b6ea1ebd61c87cbea79d444e7@peff.net","name":"Jeff King"},"message":"t: fix trivial &&-chain breakage\n\nThese are tests which are missing a link in their &&-chain,\nbut during a setup phase. We may fail to notice failure in\ncommands that build the test environment, but these are\ntypically not expected to fail at all (but it's still good\nto double-check that our test environment is what we\nexpect).\n\nSigned-off-by: Jeff King <peff@peff.net>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>","distinct":false,"url":"https://api.github.com/repos/peff/git/commits/99094a7ad484085b9f6849a8ebe1e23467c5bc14"},{"sha":"5ca812a19c97a2cbfe797748bc9bf6ebde9e0f64","author":{"email":"696e3fbcf235d40b6ea1ebd61c87cbea79d444e7@peff.net","name":"Jeff King"},"message":"t: assume test_cmp produces verbose output\n\nSome tests call test_cmp, and if it fails show the actual\noutput generated. This is mostly pointless, as test_cmp will\nalready show a diff between the expected and actual output.\nIt also fools --chain-lint by putting an \"||\" in the middle\nof the chain, so we'd rather not use this construct.\n\nNote that these cases actually show a pre-processed version\nof the data, rather than exactly what test_cmp would show.\nHowever, test_cmp's output is generally good for pointing\nthe user in the right direction, and they can then dig in\nthe trash directory themselves if they want to see more\ndetails.\n\nSigned-off-by: Jeff King <peff@peff.net>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>","distinct":false,"url":"https://api.github.com/repos/peff/git/commits/5ca812a19c97a2cbfe797748bc9bf6ebde9e0f64"},{"sha":"a167ece0c8cd9470427f8e9a1b43b27303eefa75","author":{"email":"696e3fbcf235d40b6ea1ebd61c87cbea79d444e7@peff.net","name":"Jeff King"},"message":"t: use verbose instead of hand-rolled errors\n\nMany tests that predate the \"verbose\" helper function use a\npattern like:\n\n  test ... || {\n\t  echo ...\n\t  false\n  }\n\nto give more verbose output. Using the helper, we can do\nthis with a single line, and avoid a || which interacts\nbadly with &&-chaining (besides fooling --chain-lint, we hit\nthe error block no matter which command in the chain failed,\nso we may often show useless results).\n\nIn most cases, the messages printed by \"verbose\" are equally\ngood (in some cases better; t6006 accidentally redirects the\nmessage to a file!). The exception is t7001, whose output\nsuffers slightly. However, it's still enough to show the\nuser which part failed, given that we will have just printed\nthe test script to stderr.\n\nSigned-off-by: Jeff King <peff@peff.net>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>","distinct":false,"url":"https://api.github.com/repos/peff/git/commits/a167ece0c8cd9470427f8e9a1b43b27303eefa75"},{"sha":"0a5e3c50de60c73782f1add9eb8d1e8dc8eabcb8","author":{"email":"696e3fbcf235d40b6ea1ebd61c87cbea79d444e7@peff.net","name":"Jeff King"},"message":"t: use test_must_fail instead of hand-rolled blocks\n\nThese test scripts likely predate test_must_fail, and can be\nmade simpler by using it (in addition to making them pass\n--chain-lint).\n\nThe case in t6036 loses some verbosity in the failure case,\nbut it is so tied to a specific failure mode that it is not\nworth keeping around (and the outcome of the test is not\naffected at all).\n\nSigned-off-by: Jeff King <peff@peff.net>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>","distinct":false,"url":"https://api.github.com/repos/peff/git/commits/0a5e3c50de60c73782f1add9eb8d1e8dc8eabcb8"},{"sha":"a6a4a88af0b99fec688f66256a2c23a15e110c1f","author":{"email":"696e3fbcf235d40b6ea1ebd61c87cbea79d444e7@peff.net","name":"Jeff King"},"message":"t: fix &&-chaining issues around setup which might fail\n\nMany tests have an initial setup step that might fail based\non whether earlier tests in the script have succeeded or\nnot. Using a trick like \"|| true\" breaks the &&-chain,\nmissing earlier failures (and fooling --chain-lint).\n\nWe can use test_might_fail in some cases, which is correct\nand makes the intent more obvious. We can also use\ntest_unconfig for unsetting config (and which is more\nrobust, as well).\n\nThe case in t9500 is an oddball. It wants to run cmd1 _or_\ncmd2, and does it like:\n\n  cmd1 || cmd2 &&\n  other_stuff\n\nIt's not wrong in this case, but it's a bad habit to get\ninto, because it breaks the &&-chain if used anywhere except\nat the beginning of the test (and we use the correct\nsolution here, putting it inside a block for precedence).\n\nSigned-off-by: Jeff King <peff@peff.net>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>","distinct":false,"url":"https://api.github.com/repos/peff/git/commits/a6a4a88af0b99fec688f66256a2c23a15e110c1f"},{"sha":"35da1bf5d6d51fe62d4d646c306c5cf5182a881e","author":{"email":"696e3fbcf235d40b6ea1ebd61c87cbea79d444e7@peff.net","name":"Jeff King"},"message":"t: use test_might_fail for diff and grep\n\nSome tests run diff or grep to produce an output, and then\ncompare the output to an expected value. We know the exit\ncode we expect these processes to have (e.g., grep yields 0\nif it produced output and 1 otherwise), so it would not make\nthe test wrong to look for it. But the difference between\ntheir output and the expected output (e.g., shown by\ntest_cmp) is much more useful to somebody debugging the test\nthan the test just bailing out.\n\nThese tests break the &&-chain to skip the exit-code check\nof the process. However, we can get the same effect by using\ntest_might_fail. Note that in some cases the test did use\n\"|| return 1\", which meant the test was not wrong, but it\ndid fool --chain-lint.\n\nSigned-off-by: Jeff King <peff@peff.net>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>","distinct":false,"url":"https://api.github.com/repos/peff/git/commits/35da1bf5d6d51fe62d4d646c306c5cf5182a881e"},{"sha":"c21fc9d0ab217d6e3cf01ee895e827b72d8207b6","author":{"email":"696e3fbcf235d40b6ea1ebd61c87cbea79d444e7@peff.net","name":"Jeff King"},"message":"t: use test_expect_code instead of hand-rolled comparison\n\nThis makes our output in the event of a failure slightly\nnicer, and it means that we do not break the &&-chain.\n\nSigned-off-by: Jeff King <peff@peff.net>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>","distinct":false,"url":"https://api.github.com/repos/peff/git/commits/c21fc9d0ab217d6e3cf01ee895e827b72d8207b6"},{"sha":"9ddc5ac97ecdd03ceda4b6e3ff20a7524a9f0f8d","author":{"email":"696e3fbcf235d40b6ea1ebd61c87cbea79d444e7@peff.net","name":"Jeff King"},"message":"t: wrap complicated expect_code users in a block\n\nIf we are expecting a command to produce a particular exit\ncode, we can use test_expect_code. However, some cases are\nmore complicated, and want to accept one of a range of exit\ncodes. For these, we end up with something like:\n\n  cmd;\n  case \"$?\" in\n  ...\n\nThat unfortunately breaks the &&-chain and fools\n--chain-lint. Since these special cases are so few, we can\nwrap them in a block, like this:\n\n  { cmd; ret=$?; } &&\n  case \"$ret\" in\n  ...\n\nThis accomplishes the same thing, and retains the &&-chain\n(the exit status fed to the && is that of the assignment,\nwhich should always be true). It's technically longer, but\nit is probably a good thing for unusual code like this to\nstand out.\n\nSigned-off-by: Jeff King <peff@peff.net>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>","distinct":false,"url":"https://api.github.com/repos/peff/git/commits/9ddc5ac97ecdd03ceda4b6e3ff20a7524a9f0f8d"},{"sha":"53350a35a3c29417dfc64ae2b0d7a16ba43d1239","author":{"email":"696e3fbcf235d40b6ea1ebd61c87cbea79d444e7@peff.net","name":"Jeff King"},"message":"t: avoid using \":\" for comments\n\nThe \":\" is not a comment marker, but rather a noop command.\nUsing it as a comment like:\n\n  : do something\n  cmd1 &&\n\n  : something else\n  cmd2\n\nbreaks the &&-chain, and we would fail to notice if \"cmd1\"\nfailed in this instance. We can just use regular \"#\"\ncomments instead.\n\nSigned-off-by: Jeff King <peff@peff.net>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>","distinct":false,"url":"https://api.github.com/repos/peff/git/commits/53350a35a3c29417dfc64ae2b0d7a16ba43d1239"},{"sha":"27a6625b13d7475d28f425adb2b7be221cf3c8e0","author":{"email":"696e3fbcf235d40b6ea1ebd61c87cbea79d444e7@peff.net","name":"Jeff King"},"message":"t3600: fix &&-chain breakage for setup commands\n\nAs with the earlier patch to fix \"trivial\" &&-chain\nbreakage, these missing \"&&\" operators are not a serious\nproblem (e.g., we do not expect \"echo\" to fail).\n\nIronically, however, inserting them shows that some of the\ncommands _do_ fail. Specifically, some of the tests start by\nmaking sure we are at a commit with the string \"content\" in\nthe file \"foo\". However, running \"git commit\" may fail\nbecause the previous test left us in that state already, and\nthere is nothing to commit.\n\nWe could remove these commands entirely, but they serve to\ndocument the test's assumptions, as well as make it robust\nwhen an earlier test has failed. We could use test_might_fail\nto handle all cases, but that would miss an unrelated\nfailure to make the commit. Instead, we can just pass the\n--allow-empty flag to git-commit, which means that it will\nnot complain if our setup is a noop.\n\nSigned-off-by: Jeff King <peff@peff.net>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>","distinct":false,"url":"https://api.github.com/repos/peff/git/commits/27a6625b13d7475d28f425adb2b7be221cf3c8e0"},{"sha":"aef591a0f91a80d2b39c5344653861967d70d720","author":{"email":"696e3fbcf235d40b6ea1ebd61c87cbea79d444e7@peff.net","name":"Jeff King"},"message":"t7201: fix &&-chain breakage\n\nOne of these breakages is in setup, but one is more severe\nand may miss a real test failure. These are pulled out from\nthe rest, though, because we also clean up a few other\nanachronisms. The most interesting is the use of this\nhere-doc construct:\n\n  (cat >... <<EOF\n  ...\n  EOF\n  ) &&\n\nIt looks like an attempt to make the &&-chaining more\nnatural by letting it come at the end of the here-doc. But\nthe extra sub-shell is so non-idiomatic (plus the lack of\n\"<<-\") that it ends up confusing.\n\nSince these are just using a single line, we can accomplish\nthe same thing with a single printf (which also makes the\nuse of tab more obvious than the verbatim whitespace).\n\nSigned-off-by: Jeff King <peff@peff.net>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>","distinct":false,"url":"https://api.github.com/repos/peff/git/commits/aef591a0f91a80d2b39c5344653861967d70d720"},{"sha":"d8cd32792a0a8a52ff4f5f4bb74f9044bcb9a97a","author":{"email":"696e3fbcf235d40b6ea1ebd61c87cbea79d444e7@peff.net","name":"Jeff King"},"message":"t9502: fix &&-chain breakage\n\nThis script misses a trivial &&-chain in one of its tests,\nbut it also has a weird reverse: it includes an &&-chain\noutside of any test_expect block! This \"cat\" should never\nfail, but if it did, we would not notice, as it would cause\nus to skip the follow-on test entirely (which does not\nappear intentional; there are many later tests which rely on\nthis cat).\n\nLet's instead move the setup into its own test_expect_success\nblock, which is the standard practice nowadays.\n\nSigned-off-by: Jeff King <peff@peff.net>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>","distinct":false,"url":"https://api.github.com/repos/peff/git/commits/d8cd32792a0a8a52ff4f5f4bb74f9044bcb9a97a"},{"sha":"e4e6e8b4e3aeb3f7237b83f2125c9deee221ff77","author":{"email":"696e3fbcf235d40b6ea1ebd61c87cbea79d444e7@peff.net","name":"Jeff King"},"message":"t6030: use modern test_* helpers\n\nWe can get rid of a lot of hand-rolled error messages by\nusing test_must_fail and test_expect_code. The existing code\nwas careful to use \"|| return 1\" when breaking the\n&&-chain, but it did fool --chain-lint; the new code is more\nidiomatic.\n\nWe also add some uses of test_when_finished, which is less\ncryptic and more robust than putting code at the end of a\ntest. In two cases we run \"git bisect reset\" from a\nsubshell, which is a problem for test_when_finished (it\nwould not run). However, in both of these cases, we are\nperforming the tests in one-off sub-repos, so we do not need\nto clean up at all (and in fact it is nicer not to if the\nuser wants to inspect the trash directory after a failure).\n\nSigned-off-by: Jeff King <peff@peff.net>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>","distinct":false,"url":"https://api.github.com/repos/peff/git/commits/e4e6e8b4e3aeb3f7237b83f2125c9deee221ff77"},{"sha":"9157c5cb0968f1d97bf4a79b8622a9601387a368","author":{"email":"696e3fbcf235d40b6ea1ebd61c87cbea79d444e7@peff.net","name":"Jeff King"},"message":"t0020: use modern test_* helpers\n\nThis test contains a lot of hand-rolled messages to show\nwhen the test fails. We can omit most of these by using\n\"verbose\" and \"test_must_fail\". A few of them are for\nupdate-index, but we can assume it produces reasonable error\nmessages when it fails.\n\nSigned-off-by: Jeff King <peff@peff.net>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>","distinct":false,"url":"https://api.github.com/repos/peff/git/commits/9157c5cb0968f1d97bf4a79b8622a9601387a368"},{"sha":"95508a075181397ae1040e75116a42af89dc0155","author":{"email":"696e3fbcf235d40b6ea1ebd61c87cbea79d444e7@peff.net","name":"Jeff King"},"message":"t1301: use modern test_* helpers\n\nThis shortens the code and fixes some &&-chaining.\n\nSigned-off-by: Jeff King <peff@peff.net>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>","distinct":false,"url":"https://api.github.com/repos/peff/git/commits/95508a075181397ae1040e75116a42af89dc0155"},{"sha":"2f69de5b4be64100d919b2ac858ca60263a59bb2","author":{"email":"696e3fbcf235d40b6ea1ebd61c87cbea79d444e7@peff.net","name":"Jeff King"},"message":"t6034: use modern test_* helpers\n\nThese say roughly the same thing as the hand-rolled\nmessages. We do lose the \"merge did not complete\" debug\nmessage, but merge and write-tree are prefectly capable of\nwriting useful error messages when they fail.\n\nSigned-off-by: Jeff King <peff@peff.net>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>","distinct":false,"url":"https://api.github.com/repos/peff/git/commits/2f69de5b4be64100d919b2ac858ca60263a59bb2"},{"sha":"e7d053ddb97f9a5a14567d9311dc6d8badf73a9c","author":{"email":"696e3fbcf235d40b6ea1ebd61c87cbea79d444e7@peff.net","name":"Jeff King"},"message":"t4117: use modern test_* helpers\n\nWe can use test_must_fail and test_path_* to avoid some\nhand-rolled if statements. This makes the code shorter, and\nmakes it more obvious when we are breaking the &&-chain.\n\nSigned-off-by: Jeff King <peff@peff.net>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>","distinct":false,"url":"https://api.github.com/repos/peff/git/commits/e7d053ddb97f9a5a14567d9311dc6d8badf73a9c"}]}