From: Jeff King Date: Fri, 28 Feb 2014 08:37:19 GMT Subject: Re: [PATCH 1/2] t3200-branch: test setting branch as own upstream Message-ID: <20140228083718.GA11480@sigill.intra.peff.net> In-Reply-To: On Fri, Feb 28, 2014 at 04:28:38PM +0900, Brian Gesiak wrote: > I would be in favor of using test_i18ngrep, but it seems like this > test file overwhelmingly uses test_(i18n)cmp, even when inspecting > stderr output. We generally prefer "cmp" checks to "grep" checks, because they are more rigorous. However, when testing human-readable output which may change, sometimes being too specific can simply make the tests brittle and annoying. Using a forgiving regex that matches keywords can be helpful. So there's definitely some room for judgement. I think what you posted as v2 looks OK. > Making double-sure that all tests pass when run with "sh -x" seems > like a larger endeavor. > > Of course, I'd be happy to submit several patches if there's support > for such a change. But as Peff points out it will be a large diff. Yeah, I don't think it's worth the effort. If you feel like continuing on this series, converting the warning() into a die() would be a much more productive use of time (but if you don't, I do not see any reason not to take the patches you've posted). -Peff