Re: [PATCH 1/2] t9400-git-cvsserver-server: don't rely on the output of 'test_cmp'

2018-03-08 Thread SZEDER Gábor
On Thu, Mar 8, 2018 at 11:01 PM, Eric Sunshine wrote: > On Thu, Mar 8, 2018 at 4:49 PM, SZEDER Gábor wrote: >> On Thu, Mar 8, 2018 at 7:51 PM, Eric Sunshine >> wrote: >>> An alternative approach used elsewhere in the test

Re: [PATCH 1/2] t9400-git-cvsserver-server: don't rely on the output of 'test_cmp'

2018-03-08 Thread Eric Sunshine
On Thu, Mar 8, 2018 at 5:01 PM, Eric Sunshine wrote: > Sorry for the confusion. I meant "return 1" as used elsewhere in the > test suite[1]. > [1]: For example, the "setup" test of t4151-am-abort.sh. Additional context: e6821d09e4 (t: fix some trivial cases of ignored

Re: [PATCH 1/2] t9400-git-cvsserver-server: don't rely on the output of 'test_cmp'

2018-03-08 Thread Eric Sunshine
On Thu, Mar 8, 2018 at 4:49 PM, SZEDER Gábor wrote: > On Thu, Mar 8, 2018 at 7:51 PM, Eric Sunshine wrote: >> An alternative approach used elsewhere in the test suite[1] would be >> simply to 'exit' if test_cmp fails: >> >> for i in merge no-lf

Re: [PATCH 1/2] t9400-git-cvsserver-server: don't rely on the output of 'test_cmp'

2018-03-08 Thread SZEDER Gábor
On Thu, Mar 8, 2018 at 7:51 PM, Eric Sunshine wrote: > On Thu, Mar 8, 2018 at 7:38 AM, SZEDER Gábor wrote: >> Unroll that for loop, so we can check the files' contents the usual >> way and rely on 'test_cmp's exit code failing the && chain.

Re: [PATCH 1/2] t9400-git-cvsserver-server: don't rely on the output of 'test_cmp'

2018-03-08 Thread Eric Sunshine
On Thu, Mar 8, 2018 at 7:38 AM, SZEDER Gábor wrote: > The test 'cvs update (-p)' redirects and checks 'test_cmp's stdout and > even its stderr. The commit introducing this test in 6e8937a084 > (cvsserver: Add test for update -p, 2008-03-27) doesn't discuss why, > in fact

[PATCH 1/2] t9400-git-cvsserver-server: don't rely on the output of 'test_cmp'

2018-03-08 Thread SZEDER Gábor
The test 'cvs update (-p)' redirects and checks 'test_cmp's stdout and even its stderr. The commit introducing this test in 6e8937a084 (cvsserver: Add test for update -p, 2008-03-27) doesn't discuss why, in fact its log message only consists of that subject line. Anyway, weird as it is, it kind