Re: [PATCH v2 06/10] t7006: add tests for how git tag paginates

On 31 July 2017 at 18:37, Junio C Hamano [off-list ref] wrote:

Jeff King [off-list ref] writes:

quoted

But here...

quoted

+test_expect_success TTY 'git tag -a respects pager.tag' '
+    test_when_finished "git tag -d newtag" &&
+    rm -f paginated.out &&
+    test_terminal git -c pager.tag tag -am message newtag &&
+    test -e paginated.out
+'

I think this behavior is just buggy, and it might be better introduced
as a test_expect_failure on "git tag -a does not respect pager.tag".

Kind of a minor nit, as the series should end up in the right place
either way, but it can be helpful if you end up digging back in history
to the introduction of the test.

Yes, I think that is essentially the same reaction I had to patches
7 and 8, where it carries the "buggy" behaviour forward and then
fixes it.  The way the series lays groundwork to introduce a
mechanism that can be used to address this behaviour in its earlier
patches strongly suggests to the users that this is considered a bug
by the author of the series to the user from early on, so adding
this as "expect failure" and then flip it to "expect success" when
the bug is fixed would be a more natural sequence of changes.

Thanks both for very helpful comments. I admit I viewed it less as
"fix buggy behavior" and more like "redefine wanted behavior". So I
wanted to postpone the redefinition of the behavior until all the
restructuring was done. Looking at this as a bug-fix does make
carefully moving the bug forward look rather silly.

I haven't responded to each of your suggestions individually where the
answers would have been a mere "thanks, will do". They're still much
appreciated and will help make v3 much better. Thanks.

Martin