On Sun, Aug 25, 2013 at 1:55 AM, Jonathan Nieder <jrnieder@xxxxxxxxx> wrote: > Hi, > > Eric Sunshine wrote: > >> 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. > > [...] >> --- 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? > [...] >> 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html