"Glen Choo via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: Not commenting on the code yet as I am in the middle of today's integration run, but as I notice a bad pattern being followed, let me comment before it spreads too widely. The "add failing test first and then fix the code with flipping the test to success" is very much unwelcome. For whoever gets curious enough (me included when accepting posted patch), it is easy to revert only the part of the commit outside t/ tentatively to see how the original code breaks. Keeping the fix and protection of the fix together will avoid mistakes. The post context of the hunk that changes test_expect_failure to test_expect_success does not cover the test script, thereby hiding the body of the test that changes behaviour while reviewing the patch text, which is another downside. > diff --git a/t/t0033-safe-directory.sh b/t/t0033-safe-directory.sh > index 720d6cdd60b..dc3496897ab 100755 > --- a/t/t0033-safe-directory.sh > +++ b/t/t0033-safe-directory.sh > @@ -71,7 +71,7 @@ test_expect_success 'safe.directory=*, but is reset' ' > expect_rejected_dir > ' > > -test_expect_failure 'safe.directory in included file' ' > +test_expect_success 'safe.directory in included file' ' > cat >gitconfig-include <<-EOF && > [safe] > directory = "$(pwd)" > diff --git a/t/t0035-safe-bare-repository.sh b/t/t0035-safe-bare-repository.sh > index aa6a6a8c3fd..fa33839704b 100755 > --- a/t/t0035-safe-bare-repository.sh > +++ b/t/t0035-safe-bare-repository.sh > @@ -51,7 +51,7 @@ test_expect_success 'safe.bareRepository on the command line' ' > -c safe.bareRepository=all > ' > > -test_expect_failure 'safe.bareRepository in included file' ' > +test_expect_success 'safe.bareRepository in included file' ' > cat >gitconfig-include <<-EOF && > [safe] > bareRepository = explicit
