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