Summary: | Various issues with auth_values_input_www.t | ||
---|---|---|---|
Product: | Koha | Reporter: | Mark Tompsett <mtompset> |
Component: | Test Suite | Assignee: | Mark Tompsett <mtompset> |
Status: | In Discussion --- | QA Contact: | Galen Charlton <gmcharlt> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | jonathan.druart, m.de.rooy |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17920 | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Attachments: |
Bug 17934: Make t/db_dependent/www/auth_values_input_www.t awesome
[SIGNED-OFF] Bug 17934: Make t/db_dependent/www/auth_values_input_www.t awesome |
Description
Mark Tompsett
2017-01-18 15:30:38 UTC
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? |