Hi Gábor and Dscho
On 25/06/2019 12:31, SZEDER Gábor wrote:

On Tue, Jun 25, 2019 at 11:08:04AM +0100, Phillip Wood wrote:

quoted

quoted

quoted

Rebasing (1/4)QRebasing (2/4)QRebasing (3/4)QRebasing (4/4)QQ                                                                                QSuccessfully rebased and updated refs/heads/missing-commit.

quoted

quoted

quoted

May I please *strongly* suggest to fix this first? It should

- completely lose that last line,
- be inserted into the test case itself so that e.g. disk full problems are
  caught and logged properly, and
- the `test_i18ncmp expect actual` call in the test case should be replaced
  by something like:

	sed "\$d" <actual >actual-skip-progress &&
	test_i18ncmp expect actual-skip-progress

This should obviously be made as a separate, introductory patch (probably with
a less scathing commit message than my comments above would suggest).

And that would also remove the double-yuck.

Unfortunately, this addresses only one of the "Yuck"s; see v3 of this
patch series [1].

The other yucks affect the following four tests in
't3420-rebase-autostash.sh':

  16 - rebase --merge --autostash: check output
  23 - rebase --merge: check output with conflicting stash
  26 - rebase --interactive --autostash: check output
  33 - rebase --interactive: check output with conflicting stash

These tests come from commits b76aeae553 (rebase: add regression tests
for console output, 2017-06-19) and 7d70e6b902 (rebase: add more
regression tests for console output, 2017-06-19), and are specifically
about checking the (whole) console output of 'git rebase', so I left
the updates to them as they were.

In any case, Cc-ing Phillip to discuss whether something could be done
about them (now perhaps preferably (for me :) as a follow-up, and not
another preparatory patches).

Those tests were added to check that `git stash` was being silenced (see
79a6226981 ("rebase -i: silence stash apply", 2017-05-18)).

Hmm, so would it then be possible/sensible to do something like this
instead:

  git rebase --options... >out &&
  test_i18ngrep ! "<something specific that only 'git stash' would print if it wasn't silenced>" out

quoted

I can have a
think about a better way to do that, but is it still a problem? I just
tried to take a look at your CI output and
https://dev.azure.com/gitgitgadget/git/_build/results?buildId=11406
seems to be all green - have I missed something or has Gábor fixed the
issue?

No, it was Dscho who fixed the Azure CI issue, see

  https://public-inbox.org/git/nycvar.QRO.7.76.6.1906141356140.44@tvgsbejvaqbjf.bet/

elsewhere in this thread (it's a long one, but well worth the read
IMO).

However, the point here is not that Azure CI failure, but rather the
fact that tests had to be updated to include the new line clearing
sequence in the expected output, and that "Q<...80 spaces...>Q" looks
yuck indeed.

I've come up with a sed based solution to remove the progress
notifications from the output - see
https://github.com/gitgitgadget/git/pull/276 If you're both happy with
the basic idea I'll clean it up and submit it (I've just noticed I left
some debugging bits in one of the tests). I was worried using "\r" in a
sed expression wouldn't be portable but the tests pass on gitgitgadget.
If it proves to be a problem on other platforms we could use always tr
to convert "\r" to some unique string and use that string in the sed
expression.

Best Wishes

Phillip