Zbigniew Jędrzejewski-Szmek <zbyszek@xxxxxxxxx> writes: > Eleven tests for various combinations of a long filename and/or big > change count and ways to specify widths for diff --stat. > --- Sign-off? > Tests added in previous version of 'diff --stat: use full terminal width' > are extracted into a separate patch. The tests are usefull independently > of that patch anyway. Thanks. > +# 120 character name > +name=aaaaaaaaaa > +name=$name$name$name$name$name$name$name$name$name$name$name$name > +test_expect_success 'preparation' " > + >\"$name\" && > + git add \"$name\" && > + git commit -m message && > + echo a >\"$name\" && > + git commit -m message \"$name\" > +" Just for future reference. The last parameter to test_expect_success shell function is `eval`ed by the shell and $name is visible inside it; you do not have to use double quote around it and then use backquote to quote the inner double quote. > +cat >expect <<'EOF' > + ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1 + > +EOF > +test_expect_success 'format patch graph width defaults to 80 columns' ' > + git format-patch --stat --stdout -1 >output && > + grep " | " output >actual && > + test_cmp expect actual > +' Hrm, this does not seem to pass, making the result of applying [1/3] fail; I see that the elided name is shown much shorter than the above expects. Perhaps this test found a bug in a "very long name, small changes" corner case? If that is the case, we'd mark it as test_expect_failure, and explain the tests that expect failure demonstrates a buggy behaviour, e.g. When a pathname is so long that it cannot fit on the column, the current code truncates it to make sure that the graph part has at least N columns (enough room to show a meaningful graph). If the actual change is small (e.g. only one line changed), this results in the final output that is shorter than the width we aim for. A couple of new tests marked with test_expect_failure demonstrate this bug. in the log message (note that I am not sure if that is the nature of the bug, or what the actual value of N is). And then a later patch [2/3] that updates the allocation between name and graph will turn test_expect_failure to test_expect_success; that will make it clear that your update fixed the bug. > +cat >expect <<'EOF' > + ...aaaaaaaaaaaaaaaaaaaaaaaaaa | 1 + > +EOF > +test_expect_success 'format patch --stat=width with long name' ' > + git format-patch --stat=40 --stdout -1 >output && > + grep " | " output >actual && > + test_cmp expect actual > +' Likewise. > +test_expect_success 'format patch --stat-width=width works with long name' ' > + git format-patch --stat-width=40 --stdout -1 >output && > + grep " | " output >actual && > + test_cmp expect actual > +' Likewise. > +test_expect_success 'format patch --stat=...,name-width with long name' ' > + git format-patch --stat=60,29 --stdout -1 >output && > + grep " | " output >actual && > + test_cmp expect actual > +' > + > +test_expect_success 'format patch --stat-name-width with long name' ' > + git format-patch --stat-name-width=29 --stdout -1 >output && > + grep " | " output >actual && > + test_cmp expect actual > +' Likewise. > +test_expect_success 'preparation' ' There was another "preparation" in this script already, which originally threw me off while chasing which part of the test is failing. Can you reword/retitle this one? > +cat >expect <<'EOF' > + abcd | 1000 ++++++++++++++++++++++++++++++++++++++++ > +EOF > +test_expect_success 'format patch graph part width is 40 columns' ' > + git format-patch --stat --stdout -1 >output && > + grep " | " output >actual && > + test_cmp expect actual > +' This test shouldn't be added in [1/3] because "cap the graph to 40-col" is a new feature that is introduced in the later step. > +test_expect_success 'format patch ignores COLUMNS' ' > + COLUMNS=200 git format-patch --stat --stdout -1 >output > + grep " | " output >actual && > + test_cmp expect actual > +' This is a good test to have in [1/3], but the expectation should not cap the graph part to 40 columns. The patch that updates diff.c to implement the cap in [2/3] should have an update to the expectation, whose diff hunk may look liks this: @@ cat >expect <<'EOF' + abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ - abcd | 1000 ++++++++++++++++++++++++++++++++++++++++ EOF That would make the effect of the patch clearer. > +cat >expect <<'EOF' > + ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 ++++++++++++ > +EOF > +test_expect_success 'format patch --stat=width with big change and long name' ' > + cp abcd aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa && > + git add aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa && > + git commit -m message && > + git format-patch --stat-width=60 --stdout -1 >output && > + grep " | " output >actual && > + test_cmp expect actual > +' > + > test_done The same comment as the previous one. Because you change the allocation to the graph part in your patch to diff.c, which hasn't happened in [1/3] yet, this should expect the existing behaviour (narrower graph) in this step, and then be updated to expect the output shown above in the [2/3] patch that changes the implementation in diff.c. -- 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