Re: [PATCH 1/3] t3404: preserve test_tick state across short SHA-1 collision test

On Sun, Aug 25, 2013 at 1:55 AM, Jonathan Nieder [off-list ref] wrote:

Hi,

Eric Sunshine wrote:

quoted

The short SHA-1 collision test requires carefully crafted commits in
order to ensure a collision at rebase time.

Yeah, this breaks the usual rule that tests should be independent
of hashing function.  But it's the best we can do, I think.

[...]

quoted

--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh

@@ -994,17 +994,23 @@ test_expect_success 'short SHA-1 setup' '
      test_when_finished "git checkout master" &&
      git checkout --orphan collide &&
      git rm -rf . &&
+     (
      unset test_tick &&
      test_commit collide1 collide &&
      test_commit --notick collide2 collide &&
      test_commit --notick "collide3 115158b5" collide collide3 collide3
+     )

Would be clearer if the code in a subshell were indented:

        (
                unset test_tick &&
                test_commit ...
        )

I considered it, but decided against it for a couple reasons:

* In this script, there already is a mix between the two styles:
indented vs. unindented.

* In this particular patch, the test_commit line creating commit3
wrapped beyond 80 columns when indented.

In v2 of the series (for which you also made the same observation),
the collide3 test_commit line is shorter, so I could have indented,
however, I left it alone since nobody complained about it (and because
there already is the mix of styles). Should this be worth a re-roll?

[...]

quoted

 test_expect_success 'short SHA-1 collide' '
      test_when_finished "reset_rebase && git checkout master" &&
      git checkout collide &&
+     (
+     unset test_tick &&
+     test_tick &&
      FAKE_COMMIT_MESSAGE="collide2 815200e" \
      FAKE_LINES="reword 1 2" git rebase -i HEAD~2
+     )

Likewise.

Hope that helps,
Jonathan