# Test 9500 - unhelpful without Perl::CGI 6 messages from 2011-06-15 to 2011-06-15. Participants: Linus Torvalds, Junio C Hamano, Sverre Rabbelier. Thread: https://gitlist.dev/t/27626 ## Linus Torvalds, 2011-06-15 02:37 Subject: Test 9500 - unhelpful without Perl::CGI Message-ID: URL: https://gitlist.dev/e/BANLkTikj0GQCN94PvZz6n6fJJb5sHMApig%40mail.gmail.com ``` This has probably been going on forever, and the only reason I noticed is that I had a machine that doesn't have perl-CGI installed. I'm talking about t9500-gitweb-standalone-no-errors. Anyway, if you don't have that, the test fails at everything, and it does so without any clues about _why_ it fails. I ended up figuring it out by doing a "strace -f" to see what seems to be wrong. Maybe that test could have some additional check for "is perl::CGI installed at all" and at least give a useful error message rather than just saying it failed? Linus ``` ## Junio C Hamano, 2011-06-15 05:59 Subject: Re: Test 9500 - unhelpful without Perl::CGI Message-ID: <7vmxhjk539.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vmxhjk539.fsf%40alter.siamese.dyndns.org In-Reply-To: ``` Linus Torvalds writes: > This has probably been going on forever, and the only reason I noticed > is that I had a machine that doesn't have perl-CGI installed. > > I'm talking about t9500-gitweb-standalone-no-errors. > > Anyway, if you don't have that, the test fails at everything, and it > does so without any clues about _why_ it fails. Something like this would hopefully suffice. It seems that we do test for the version of Encode, but we do not specifically test CGI or other modules that people do not have to slurp from CPAN? t/gitweb-lib.sh | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh index 143eb1f..292753f 100644 --- a/t/gitweb-lib.sh +++ b/t/gitweb-lib.sh @@ -107,4 +107,9 @@ perl -MEncode -e '$e="";decode_utf8($e, Encode::FB_CROAK)' >/dev/null 2>&1 || { test_done } +perl -MCGI -MCGI::Util -MCGI::Carp -e 0 >/dev/null 2>&1 || { + skip_all='skipping gitweb tests, CGI module unusable' + test_done +} + gitweb_init ``` ## Sverre Rabbelier, 2011-06-15 07:26 Subject: Re: Test 9500 - unhelpful without Perl::CGI Message-ID: URL: https://gitlist.dev/e/BANLkTinAfqysABm%2Bt_NkOGO2U%2Biy57WbVQ%40mail.gmail.com In-Reply-To: <7vmxhjk539.fsf@alter.siamese.dyndns.org> ``` Heya, On Wed, Jun 15, 2011 at 07:59, Junio C Hamano wrote: > +perl -MCGI -MCGI::Util -MCGI::Carp -e 0 >/dev/null 2>&1 || { > +       skip_all='skipping gitweb tests, CGI module unusable' > +       test_done > +} > + Isn't the new style to use test_must_succeed CGI_AVAILABLE '...' '...' rather than skip_all+test_done? -- Cheers, Sverre Rabbelier ``` ## Junio C Hamano, 2011-06-15 16:51 Subject: Re: Test 9500 - unhelpful without Perl::CGI Message-ID: <7vips7jaws.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vips7jaws.fsf%40alter.siamese.dyndns.org In-Reply-To: ``` Sverre Rabbelier writes: > On Wed, Jun 15, 2011 at 07:59, Junio C Hamano wrote: >> +perl -MCGI -MCGI::Util -MCGI::Carp -e 0 >/dev/null 2>&1 || { >> +       skip_all='skipping gitweb tests, CGI module unusable' >> +       test_done >> +} >> + > > Isn't the new style to use test_must_succeed CGI_AVAILABLE '...' '...' > rather than skip_all+test_done? I do not think that adds any value, at least in this case. Besides, look at the context line above what you quoted ;-) ``` ## Junio C Hamano, 2011-06-15 17:05 Subject: Re: Test 9500 - unhelpful without Perl::CGI Message-ID: <7vei2vja8q.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vei2vja8q.fsf%40alter.siamese.dyndns.org In-Reply-To: <7vips7jaws.fsf@alter.siamese.dyndns.org> ``` Junio C Hamano writes: > Sverre Rabbelier writes: > >> Isn't the new style to use test_must_succeed CGI_AVAILABLE '...' '...' >> rather than skip_all+test_done? > > I do not think that adds any value, at least in this case. Besides, look > at the context line above what you quoted ;-) In any case, I'd justify it like this and queue. Thanks for noticing. -- >8 -- Subject: [PATCH] t/gitweb-lib.sh: skip gitweb tests when perl dependencies are not met Linus noticed that we go ahead testing gitweb and fail miserably on a box with Perl but not perl-CGI library. We already have a code to detect lack of Perl and refrain from testing gitweb in t/gitweb-lib.sh (by the way, shouldn't it be called t/lib-gitweb.sh?), so let's extend it to cover this case as well. Signed-off-by: Junio C Hamano --- t/gitweb-lib.sh | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh index 143eb1f..292753f 100644 --- a/t/gitweb-lib.sh +++ b/t/gitweb-lib.sh @@ -107,4 +107,9 @@ perl -MEncode -e '$e="";decode_utf8($e, Encode::FB_CROAK)' >/dev/null 2>&1 || { test_done } +perl -MCGI -MCGI::Util -MCGI::Carp -e 0 >/dev/null 2>&1 || { + skip_all='skipping gitweb tests, CGI module unusable' + test_done +} + gitweb_init -- 1.7.6.rc1.119.g85fc2 ``` ## Linus Torvalds, 2011-06-15 18:36 Subject: Re: Test 9500 - unhelpful without Perl::CGI Message-ID: URL: https://gitlist.dev/e/BANLkTineU1x8NDk7Q5eUN3rkg0%3DyPH2k%3Dw%40mail.gmail.com In-Reply-To: <7vei2vja8q.fsf@alter.siamese.dyndns.org> ``` On Wed, Jun 15, 2011 at 10:05 AM, Junio C Hamano wrote: > > Subject: [PATCH] t/gitweb-lib.sh: skip gitweb tests when perl dependencies are not met Looks good. This makes t9500 go from not ok - 1 no commits: projects_list (implicit) # gitweb_run not ok - 2 no commits: projects_index # gitweb_run "a=project_index" not ok - 3 no commits: .git summary (implicit) # gitweb_run "p=.git" not ok - 4 no commits: .git commit (implicit HEAD) ... not ok - 98 categories: projects list, only default category # echo "\$projects_list_group_categories = 1;" >>gitweb_config.perl && # gitweb_run # failed 91 among 98 test(s) 1..98 to # passed all 0 test(s) 1..0 # SKIP skipping gitweb tests, CGI module unusable without perl-CGI installed. And with perl-CGI installed I obviously get ... ok 97 - ctags: malformed tag weights ok 98 - categories: projects list, only default category # passed all 98 test(s) 1..98 So Ack on that patch. Thanks, Linus ```