Re: [PATCH v2 1/3] t/gitweb-lib.sh: print to stderr when gitweb_run has errors

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Feb 19, 2011 at 19:16, Jakub Narebski <jnareb@xxxxxxxxx> wrote:
> On Sat, 19 Feb 2011, Ãvar ArnfjÃrà Bjarmason wrote:
>> On Sat, Feb 19, 2011 at 16:46, Jakub Narebski <jnareb@xxxxxxxxx> wrote:
>>> On Sat, 19 Feb 2011, Ävar ArnfjÃrÅ Bjarmason wrote:
>>>
>>>> Change the gitweb_run test subroutine to spew errors to stderr if
>>>> there are any, previously it would just silently fail, which made
>>>> tests very hard to debug.
>>>>
>>>> Before you'd get this output, when running tests under `--verbose
>>>> --immediate --debug`:
>>>
>>> Which test?
>>>
>>> [...]
>>>> --- a/t/gitweb-lib.sh
>>>> +++ b/t/gitweb-lib.sh
>>>> @@ -82,7 +82,12 @@ gitweb_run () {
>>>> Â Â Â Â Â Â Â }
>>>> Â Â Â Â Â Â Â close O;
>>>> Â Â Â ' gitweb.output &&
>>>> - Â Â if grep '^[[]' gitweb.log>/dev/null 2>&1; then false; else true; fi
>>>> + Â Â if grep '^[[]' gitweb.log>/dev/null 2>&1; then
>>>> + Â Â Â Â Â Â cat gitweb.log>&2
>>>> + Â Â Â Â Â Â false
>>>> + Â Â else
>>>> + Â Â Â Â Â Â true
>>>> + Â Â fi
>>>
>>> I don't understand this change. ÂEither it is not necessary, because
>>> test suite (or at least t9500) has
>>>
>>> Âtest_debug 'cat gitweb.log'
>>>
>>> after each test, so that error messages would be printed with `--debug`,
>>> or it doesn't go far enough: if the above is used then those test_debug
>>> should be removed.
>>
>> The way you're using test_debug() is incompatible with
>> --immediate. The test dies, but I'll never see your debug message
>> because I'm using --immediate.
>
> Ah, so that is what it is about: using --immediate negates --debug.

Yeah, I didn't realize that when I submitted my patch, I just thought
it would never be printed, period.

> Note that it is *much* wider issue; it is not only gitweb tests that use
> test_debug in such way. ÂSee for example t/t4114-apply-typechange.sh
> where you have

Indeed, it's all over the test suite.

> Âtest_expect_success SYMLINKS 'directory becomes symlink' '
> Â Â Â Âgit checkout -f foo-becomes-a-directory &&
> Â Â Â Âgit diff-tree -p HEAD foo-symlinked-to-bar > patch &&
> Â Â Â Âgit apply --index < patch
> Â Â Â Â'
> Âtest_debug 'cat patch'
>
> This causes the same problem.
>
>> It would be better to just use test_debug in gitweb_run (instead of my
>> "cat gitweb.log").
>>
>> Anyway, if you feel like fixing that feel free, I wan't pursue this
>> further (going to hack on what I was going to do before gitweb tests
>> started failing).
>>
>> Junio, you can drop this patch since it'll produce duplicate output if
>> the test fails and the user *doesn't* use --immediate, but IMO this
>> should be fixed by doing the debug output differently.
>
> Below there is proposed patch that removes duplication of --debug output...
> but does not solve issue for other places where we use test_debug in test
> suite and incompatibility with --immediate run.

This patch looks good to me.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]