Kohadevbox is the best way to duplicate this problem, as tests need to be run in a koha-shell to work, and running from the kohaclone directory owned by vagrant as kohadev means hitting them. Bug 17920 dealt with Sitemapper which I encountered. Marcel suggested in https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17920#c6 to potentially fix: t/db_dependent/SuggestionEngine_ExplodedTerms.t -- nope t/db_dependent/www/auth_values_input_www.t -- Yes (unused line) t/db_dependent/www/batch.t -- No, but some mrc test files in path might be useful t/db_dependent/www/search_utf8.t -- Hmmm, same issue as batch.t t/db_dependent/zebra_config.pl -- Why are __FOOBAR__ items in this file? This means I will fix the similar problem to Sitemapper in this bug, and push the missing MRC files and confusing file to another bug, which someone else can submit if they wish. Short version: make t/db_dependent/www/auth_values_input_www.t more awesome.
Created attachment 59173 [details] [review] Bug 17934: Make t/db_dependent/www/auth_values_input_www.t awesome There were two lines of code which declared unused variables. They were removed. Next, the issue of double planning encountered on bug 17540 (https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17540#c3) was encountered. So this corrects that and adds a little more error handling into the whole C4::Context using process so that perlcritic -4 is achieved. Also typo on Korean in a comment. TEST PLAN --------- 2) follow these commands: $ mv C4/Context.pm C4/Context_backup.pm $ sudo koha-shell -c "prove t/db_dependent/www/auth_values_input_www.t" kohadev -- Nasty fail $ mv C4/Context_backup.pm C4/Context.pm $ sudo koha-shell -c "export PERL5LIB=\"\";prove t/db_dependent/www/auth_values_input_www.t" kohadev -- Nasty fail $ sudo koha-shell -c "export KOHA_CONF=\"\";prove t/db_dependent/www/auth_values_input_www.t" kohadev -- Nasty fail $ sudo koha-shell -c "export KOHA_INTRANET_URL=\"\";prove t/db_dependent/www/auth_values_input_www.t" kohadev $ sudo koha-shell -c "prove t/db_dependent/www/auth_values_input_www.t" kohadev -- Nasty fail $ sudo koha-shell -c "export KOHA_INTRANET_URL=\"http://localhost:8080/\";prove t/db_dependent/www/auth_values_input_www.t" kohadev -- This should work. $ perlcritic -4 t/db_dependent/www/auth_values_input_www.t -- This should fail 2) apply patch 3) follow these commands: $ mv C4/Context.pm C4/Context_backup.pm $ sudo koha-shell -c "prove t/db_dependent/www/auth_values_input_www.t" kohadev -- This should trigger the C4::Context not working message $ mv C4/Context_backup.pm C4/Context.pm $ sudo koha-shell -c "export PERL5LIB=\"\";prove t/db_dependent/www/auth_values_input_www.t" kohadev -- This should trigger the C4::Contxt not loading message $ sudo koha-shell -c "export KOHA_CONF=\"\";prove t/db_dependent/www/auth_values_input_www.t" kohadev -- This should trigger the KOHA_CONF not set message $ sudo koha-shell -c "export KOHA_INTRANET_URL=\"\";prove t/db_dependent/www/auth_values_input_www.t" kohadev $ sudo koha-shell -c "prove t/db_dependent/www/auth_values_input_www.t" kohadev -- These should both trigger the KOHA_INTRANET_URL not set message $ sudo koha-shell -c "export KOHA_INTRANET_URL=\"http://localhost:8080/\";prove t/db_dependent/www/auth_values_input_www.t" kohadev -- This should work. $ perlcritic -4 t/db_dependent/www/auth_values_input_www.t -- This should be OK 3) run koha qa test tools
Comment on attachment 59173 [details] [review] Bug 17934: Make t/db_dependent/www/auth_values_input_www.t awesome Review of attachment 59173 [details] [review]: ----------------------------------------------------------------- After rereading the code changes, perhaps enhancement is better. You'll note that perlcritic -1 on this will be nice and short. ::: t/db_dependent/www/auth_values_input_www.t @@ -30,2 @@ > > -my $testdir = File::Spec->rel2abs( dirname(__FILE__) ); not used. @@ +35,5 @@ > + $skip_all = "Tests skipped. You must have a working C4::Context.\n"; > +} > +elsif ( ! can_load( modules => { 'C4::Context' => undef } ) ) { > + $skip_all = "Tests skipped. You must be able to load C4::Context.\n"; > +} Added to determine cause of failure better. @@ +45,5 @@ > + $user = $ENV{KOHA_USER} || $xml->{config}->{user}; > + $password = $ENV{KOHA_PASS} || $xml->{config}->{pass}; > +} > +elsif ($skip_all eq q{}) { > + $skip_all = "Tests skipped. You must set env. variable KOHA_CONF to run tests.\n"; Added check. @@ +56,4 @@ > } > > +if (length $skip_all > 0) { > + croak $skip_all; # to keep Test Summary output Any failures due to prerequisites will happen here. @@ +63,3 @@ > my $dbh = C4::Context->dbh; > > +$intranet =~ s/\/$//xsm; perlcritic -1 prefers /'s and xsm added. @@ -54,3 @@ > > my $agent = Test::WWW::Mechanize->new( autocheck => 1 ); > -my $jsonresponse; Not used. @@ +72,5 @@ > $agent->get_ok( "$intranet/cgi-bin/koha/mainpage.pl", 'connect to intranet' ); > $agent->form_name('loginform'); > $agent->field( 'password', $password ); > $agent->field( 'userid', $user ); > +$agent->field( 'branch', q{} ); q{} is perlcritic -1 friendly. @@ +78,3 @@ > $agent->get_ok( "$intranet/cgi-bin/koha/mainpage.pl", 'load main page' ); > > +#--------------------------------------------------- Test with Korean and greek chars Typo! @@ +88,4 @@ > $agent->get_ok( "$intranet/cgi-bin/koha/admin/authorised_values.pl?op=add_form", 'Open to create a new category' ); > $agent->form_name('Aform'); > $agent->field('category', $category); > +$agent->click_ok( q{}, 'Create new AV category ' ); Using single quotes for non-interpolated strings is perlcritic friendlier. @@ +94,3 @@ > $add_form_link_exists = 0; > for my $link ( $agent->links() ) { > + if ( $link->url =~ m/authorised_values.pl\x3Fop=add_form&category=$category/xsm ) { perlcritic didn't like \?, so I hex'd it. @@ +123,5 @@ > $agent->get_ok( "$intranet/cgi-bin/koha/admin/authorised_values.pl", 'Return to Authorized values page' ); > $agent->get_ok( "$intranet/cgi-bin/koha/admin/authorised_values.pl?searchfield=学協会μμ&offset=0", 'Search the values inserted' ); > my $text = $agent->text() ; > #Tests on UTF-8 > +ok ( ( length Encode::encode('UTF-8', $text) != length $text ) , 'UTF-8 are multi-byte. Good') ; length is a built in function, no need for brackets on it. Removed brackets for perlcritic friendliness. @@ +173,5 @@ > $delete_form_link_exists = 1; > } > } > +is( $add_form_link_exists, 1, 'Add form link exists'); > +is( $delete_form_link_exists, 1, 'Delete form link exists'); Added a reasonable test description string to make perlcritic happier.
Created attachment 59753 [details] [review] [SIGNED-OFF] Bug 17934: Make t/db_dependent/www/auth_values_input_www.t awesome There were two lines of code which declared unused variables. They were removed. Next, the issue of double planning encountered on bug 17540 (https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17540#c3) was encountered. So this corrects that and adds a little more error handling into the whole C4::Context using process so that perlcritic -4 is achieved. Also typo on Korean in a comment. TEST PLAN --------- 2) follow these commands: $ mv C4/Context.pm C4/Context_backup.pm $ sudo koha-shell -c "prove t/db_dependent/www/auth_values_input_www.t" kohadev -- Nasty fail $ mv C4/Context_backup.pm C4/Context.pm $ sudo koha-shell -c "export PERL5LIB=\"\";prove t/db_dependent/www/auth_values_input_www.t" kohadev -- Nasty fail $ sudo koha-shell -c "export KOHA_CONF=\"\";prove t/db_dependent/www/auth_values_input_www.t" kohadev -- Nasty fail $ sudo koha-shell -c "export KOHA_INTRANET_URL=\"\";prove t/db_dependent/www/auth_values_input_www.t" kohadev $ sudo koha-shell -c "prove t/db_dependent/www/auth_values_input_www.t" kohadev -- Nasty fail $ sudo koha-shell -c "export KOHA_INTRANET_URL=\"http://localhost:8080/\";prove t/db_dependent/www/auth_values_input_www.t" kohadev -- This should work. $ perlcritic -4 t/db_dependent/www/auth_values_input_www.t -- This should be OK 2) apply patch 3) follow these commands: $ mv C4/Context.pm C4/Context_backup.pm $ sudo koha-shell -c "prove t/db_dependent/www/auth_values_input_www.t" kohadev -- This should trigger the C4::Context not working message $ mv C4/Context_backup.pm C4/Context.pm $ sudo koha-shell -c "export PERL5LIB=\"\";prove t/db_dependent/www/auth_values_input_www.t" kohadev -- This should trigger the C4::Contxt not loading message $ sudo koha-shell -c "export KOHA_CONF=\"\";prove t/db_dependent/www/auth_values_input_www.t" kohadev -- This should trigger the KOHA_CONF not set message $ sudo koha-shell -c "export KOHA_INTRANET_URL=\"\";prove t/db_dependent/www/auth_values_input_www.t" kohadev $ sudo koha-shell -c "prove t/db_dependent/www/auth_values_input_www.t" kohadev -- These should both trigger the KOHA_INTRANET_URL not set message $ sudo koha-shell -c "export KOHA_INTRANET_URL=\"http://localhost:8080/\";prove t/db_dependent/www/auth_values_input_www.t" kohadev -- This should work. $ perlcritic -4 t/db_dependent/www/auth_values_input_www.t -- This should be OK 3) run koha qa test tools Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment on attachment 59753 [details] [review] [SIGNED-OFF] Bug 17934: Make t/db_dependent/www/auth_values_input_www.t awesome Review of attachment 59753 [details] [review]: ----------------------------------------------------------------- ::: t/db_dependent/www/auth_values_input_www.t @@ +167,4 @@ > $add_form_link_exists = 0; > $delete_form_link_exists = 0; > for my $link ( $agent->links() ) { > + if ( $link->url =~ m/authorised_values.pl\x3Fop=add_form&category=$category/xsm ) { Really? This is terrible. I do not see the point to follow perlcritic -1 if it's to introduce such things.
And why do you test the validity of C4::Context?