Hi Eric, On Wed, 19 Jun 2019, Johannes Schindelin wrote: > On Wed, 19 Jun 2019, Eric Sunshine wrote: > > > On Wed, Jun 19, 2019 at 2:19 AM Johannes Sixt <j6t@xxxxxxxx> wrote: > > > Am 19.06.19 um 01:12 schrieb Eric Sunshine: > > > > On Tue, Jun 18, 2019 at 8:24 AM Johannes Schindelin via GitGitGadget > > > > <gitgitgadget@xxxxxxxxx> wrote: > > > >> printf ".git\nfatal: Needed a single revision\n" >expect && > > > >> - test_cmp expect output.txt > > > >> + sort <output.txt >output.sorted && > > > >> + test_cmp expect output.sorted > > > > > > > > It was quite surprising to see this sorting only 'output' but not > > > > 'expect'. I see now that 'output' is already "sorted" (in that sense), > > > > but it feels fragile. More robust would be to sort 'expect' as well: > > > > > > > > printf ".git\nfatal: Needed a single revision\n" | sort >expect && > > > > > > Following Dscho's recent objection elsewhere that tests tend to check > > > for much more than regressions, wouldn't it be logical to write these as > > > > > > grep -F .git" output.txt && > > > test_i18n_grep "Needed a single rev" output.txt > > > > > > without an 'expect' file at all? > > > > I considered suggesting that, as well, as being more obvious and less > > fragile (with the exception that "Needed a single rev" isn't currently > > localizable in builtin/rev-parse.c, so plain 'grep' instead of > > 'test_i18n_grep'). Interesting side note: I just realized that t6050-replace.sh does indeed contain test_i18ngrep "Needed a single revision" err so I wonder why that works. If anybody has an answer, I'd be curious, but for now I want to focus on this here patch series instead. Ciao, Dscho > Valid points all around, thank you so much! > > The next iteration will have the two `grep`s instead, > Dscho >
