From 8165d07b1c31843f185e88587acc0c872335cad5 Mon Sep 17 00:00:00 2001 From: Matthieu Muffato Date: Wed, 17 Jun 2020 13:26:06 +0100 Subject: [PATCH 1/6] Test init_pipeline outside of the test process. init_pipeline was the last script tested this way. That caused some issues because it is not isolated from the test process, and for instance it will register the pipeline in TheApiary. Also, the test is more reliable if the actual script is tested. --- modules/Bio/EnsEMBL/Hive/Utils/Test.pm | 19 +++++-------------- .../fetch_and_count_by_multiple_columns.t | 2 +- t/03.scripts/beekeeper_big_red_button.t | 7 +++---- t/03.scripts/beekeeper_opts.t | 4 ++-- t/03.scripts/generate_graph.t | 18 ++++++++---------- t/10.pipeconfig/analysis_heir.t | 3 +-- t/10.pipeconfig/gc_dataflow.t | 3 +-- t/10.pipeconfig/pipeline_url.t | 11 +++++------ 8 files changed, 26 insertions(+), 41 deletions(-) diff --git a/modules/Bio/EnsEMBL/Hive/Utils/Test.pm b/modules/Bio/EnsEMBL/Hive/Utils/Test.pm index 7d014b347..fbfb772c5 100644 --- a/modules/Bio/EnsEMBL/Hive/Utils/Test.pm +++ b/modules/Bio/EnsEMBL/Hive/Utils/Test.pm @@ -199,20 +199,11 @@ sub init_pipeline { $url = (splice(@$options, $url_flag_index, 2))[1]; } - local @ARGV = @$options; - unshift @ARGV, (-pipeline_url => $url, -hive_force_init => 1); - - lives_ok(sub { - my $orig_unambig_url = Bio::EnsEMBL::Hive::Utils::URL::parse($url)->{'unambig_url'}; - ok($orig_unambig_url, 'Given URL could be parsed'); - my $returned_url = Bio::EnsEMBL::Hive::Scripts::InitPipeline::init_pipeline($file_or_module, $tweaks); - ok($returned_url, 'pipeline initialized on '.$returned_url); - - my $returned_unambig_url = Bio::EnsEMBL::Hive::Utils::URL::parse($returned_url)->{'unambig_url'}; - # Both $url and $returned_url MAY contain the password (if applicable for the driver) but can be missing the port number assuming a default - # Both $orig_unambig_url and $returned_unambig_url SHOULD contain the port number (if applicable for the driver) but WILL NOT contain a password - is($returned_unambig_url, $orig_unambig_url, 'pipeline initialized on '.$url); - }, sprintf('init_pipeline("%s", %s)', $file_or_module, stringify($options))); + my @args = ($file_or_module, -pipeline_url => $url, -hive_force_init => 1); + push @args, @$options; + push @args, map {-tweak => $_} @$tweaks if $tweaks; + + return _test_ehive_script('init_pipeline', undef, \@args); } diff --git a/t/02.api/fetch_and_count_by_multiple_columns.t b/t/02.api/fetch_and_count_by_multiple_columns.t index b4da6290a..18d3bb3e7 100755 --- a/t/02.api/fetch_and_count_by_multiple_columns.t +++ b/t/02.api/fetch_and_count_by_multiple_columns.t @@ -37,7 +37,7 @@ my $ehive_test_pipeline_urls = get_test_urls(); foreach my $pipeline_url (@$ehive_test_pipeline_urls) { subtest 'Test on '.$pipeline_url, sub { - plan tests => 20; + plan tests => 17; init_pipeline('Bio::EnsEMBL::Hive::Examples::LongMult::PipeConfig::LongMult_conf', $pipeline_url); diff --git a/t/03.scripts/beekeeper_big_red_button.t b/t/03.scripts/beekeeper_big_red_button.t index 476bbbfe8..0e275ff4c 100755 --- a/t/03.scripts/beekeeper_big_red_button.t +++ b/t/03.scripts/beekeeper_big_red_button.t @@ -44,10 +44,9 @@ my $test_filename = 'foo'; my $pipeline_url = get_test_url_or_die(); { - # The init_pipeline test runs in the same process, so @INC needs - # to be updated to see the test modules - local @INC = @INC; - push @INC, $local_module_path; + # PERL5LIB needs to be updated in order to see + # TestPipeConfig::LongWorker_conf and TestRunnable::DummyWriter + local $ENV{'PERL5LIB'} = "$local_module_path:" . $ENV{'PERL5LIB'}; init_pipeline( 'TestPipeConfig::LongWorker_conf', $pipeline_url, diff --git a/t/03.scripts/beekeeper_opts.t b/t/03.scripts/beekeeper_opts.t index a122b0631..8c82c109c 100755 --- a/t/03.scripts/beekeeper_opts.t +++ b/t/03.scripts/beekeeper_opts.t @@ -34,8 +34,8 @@ my $pipeline_url = get_test_url_or_die(); # Starting a first set of checks with a "GCPct" pipeline { - local @INC = @INC; - push @INC, $ENV{'EHIVE_ROOT_DIR'}.'/t/03.scripts/'; + # PERL5LIB needs to be updated in order to see TestPipeConfig::LongWorker_conf + local $ENV{'PERL5LIB'} = $ENV{'EHIVE_ROOT_DIR'}.'/t/03.scripts/' . ':' . $ENV{'PERL5LIB'}; init_pipeline('TestPipeConfig::LongWorker_conf', $pipeline_url); } diff --git a/t/03.scripts/generate_graph.t b/t/03.scripts/generate_graph.t index 3a277fba5..07c7bdf7d 100755 --- a/t/03.scripts/generate_graph.t +++ b/t/03.scripts/generate_graph.t @@ -39,10 +39,14 @@ GetOptions( my $server_url = get_test_url_or_die(-tag => 'server', -no_user_prefix => 1); -# Most of the test scripts test init_pipeline() from Utils::Test but we -# also need to test the main scripts/init_pipeline.pl ! -my @init_pipeline_args = ($ENV{'EHIVE_ROOT_DIR'}.'/scripts/init_pipeline.pl', 'Bio::EnsEMBL::Hive::Examples::LongMult::PipeConfig::LongMultServer_conf', -pipeline_url => $server_url, -hive_force_init => 1, -tweak => 'pipeline.param[take_time]=0'); -test_command(\@init_pipeline_args); +init_pipeline( + 'Bio::EnsEMBL::Hive::Examples::LongMult::PipeConfig::LongMultServer_conf', + $server_url, + [], + [ + 'pipeline.param[take_time]=0', + ], +); my $client_url = get_test_url_or_die(-tag => 'client', -no_user_prefix => 1); @@ -59,12 +63,6 @@ push @confs_to_test, 'GC::PipeConfig::GCPct_conf' unless $@; # SKIP it in cas my ($fh, $tmp_filename) = tempfile(UNLINK => 1); close($fh); -sub test_command { - my $cmd_array = shift; - ok(!system(@$cmd_array), 'Can run '.join(' ', @$cmd_array)); -} - - foreach my $conf (@confs_to_test) { subtest $conf, sub { my $module_name = 'Bio::EnsEMBL::Hive::Examples::'.$conf; diff --git a/t/10.pipeconfig/analysis_heir.t b/t/10.pipeconfig/analysis_heir.t index 7e516c024..79ab9c190 100755 --- a/t/10.pipeconfig/analysis_heir.t +++ b/t/10.pipeconfig/analysis_heir.t @@ -32,8 +32,7 @@ $ENV{'EHIVE_ROOT_DIR'} ||= File::Basename::dirname( File::Basename::dirname( Fil my $pipeline_url = get_test_url_or_die(); my $init_stderr = capture_stderr { - local @INC = @INC; - push @INC, $ENV{'EHIVE_ROOT_DIR'}.'/t/10.pipeconfig/'; + local $ENV{'PERL5LIB'} = $ENV{'EHIVE_ROOT_DIR'}.'/t/10.pipeconfig/::'.$ENV{PERL5LIB}; init_pipeline( 'TestPipeConfig::MissingAnalysis_conf', $pipeline_url, diff --git a/t/10.pipeconfig/gc_dataflow.t b/t/10.pipeconfig/gc_dataflow.t index bf7d5cbb7..5d40e8051 100755 --- a/t/10.pipeconfig/gc_dataflow.t +++ b/t/10.pipeconfig/gc_dataflow.t @@ -30,8 +30,7 @@ $ENV{'EHIVE_ROOT_DIR'} ||= File::Basename::dirname( File::Basename::dirname( Fil my $pipeline_url = get_test_url_or_die(); { - local @INC = @INC; - push @INC, $ENV{'EHIVE_ROOT_DIR'}.'/t/10.pipeconfig/'; + local $ENV{'PERL5LIB'} = $ENV{'EHIVE_ROOT_DIR'}.'/t/10.pipeconfig/::'.$ENV{PERL5LIB}; init_pipeline('TestPipeConfig::AnyFailureBranch_conf', $pipeline_url, [], ['pipeline.param[take_time]=100']); } my $hive_dba = Bio::EnsEMBL::Hive::DBSQL::DBAdaptor->new( -url => $pipeline_url ); diff --git a/t/10.pipeconfig/pipeline_url.t b/t/10.pipeconfig/pipeline_url.t index 46a4d0683..4dc4032af 100755 --- a/t/10.pipeconfig/pipeline_url.t +++ b/t/10.pipeconfig/pipeline_url.t @@ -21,7 +21,7 @@ use warnings; use Test::More; -use Bio::EnsEMBL::Hive::Utils::Test qw(init_pipeline beekeeper get_test_url_or_die safe_drop_database); +use Bio::EnsEMBL::Hive::Utils::Test qw(init_pipeline get_test_url_or_die safe_drop_database); use Bio::EnsEMBL::Hive::Utils qw(destringify); # eHive needs this to initialize the pipeline (and run db_cmd.pl) @@ -29,11 +29,10 @@ $ENV{'EHIVE_ROOT_DIR'} ||= File::Basename::dirname( File::Basename::dirname( Fil my $pipeline_url = get_test_url_or_die(); -# Utils::Test::init_pipeline runs things internally instead of invoking init_pipeline.pl -# Here we need to run the script to trigger the password sanitization -my @cmd = ('env', 'PERL5LIB='.$ENV{'EHIVE_ROOT_DIR'}.'/t/10.pipeconfig/::'.$ENV{PERL5LIB}, $ENV{'EHIVE_ROOT_DIR'}.'/scripts/init_pipeline.pl', 'TestPipeConfig::PipelineURL_conf', -pipeline_url => $pipeline_url); -my $rc = system(@cmd); -is($rc, 0, 'init_pipeline.pl successfully ran'); +{ + local $ENV{'PERL5LIB'} = $ENV{'EHIVE_ROOT_DIR'}.'/t/10.pipeconfig/::'.$ENV{PERL5LIB}; + init_pipeline('TestPipeConfig::PipelineURL_conf', $pipeline_url); +} my $hive_dba = Bio::EnsEMBL::Hive::DBSQL::DBAdaptor->new( -url => $pipeline_url ); From cc48f80ad34390f96cb22714aea887890b5e08db Mon Sep 17 00:00:00 2001 From: Matthieu Muffato Date: Wed, 17 Jun 2020 13:31:20 +0100 Subject: [PATCH 2/6] Added an option to expect failures when testing scripts --- modules/Bio/EnsEMBL/Hive/Utils/Test.pm | 45 ++++++++++++++++++++++---- t/02.api/create_drop_database.t | 2 +- t/03.scripts/beekeeper_opts.t | 29 +++++++---------- 3 files changed, 50 insertions(+), 26 deletions(-) diff --git a/modules/Bio/EnsEMBL/Hive/Utils/Test.pm b/modules/Bio/EnsEMBL/Hive/Utils/Test.pm index fbfb772c5..6369d2156 100644 --- a/modules/Bio/EnsEMBL/Hive/Utils/Test.pm +++ b/modules/Bio/EnsEMBL/Hive/Utils/Test.pm @@ -166,6 +166,8 @@ sub standaloneJob { Arg[2] : String $url. The location of the database to be created Arg[3] : (optional) Arrayref $args. Extra parameters of the pipeline (as on the command-line) Arg[4] : (optional) Arrayref $tweaks. Tweaks to be applied to the database (as with the -tweak command-line option) + Arg[5] : (optional) Hashref $flags. Flags given to the text framework. Currently the only + key that is understood is "expect_failure" to reverse the expectation of the test. Example : init_pipeline( 'Bio::EnsEMBL::Hive::Examples::LongMult::PipeConfig::LongMultServer_conf', $server_url, @@ -183,7 +185,7 @@ sub standaloneJob { =cut sub init_pipeline { - my ($file_or_module, $url, $options, $tweaks) = @_; + my ($file_or_module, $url, $options, $tweaks, $flags) = @_; $options ||= []; @@ -203,7 +205,7 @@ sub init_pipeline { push @args, @$options; push @args, map {-tweak => $_} @$tweaks if $tweaks; - return _test_ehive_script('init_pipeline', undef, \@args); + return _test_ehive_script('init_pipeline', undef, \@args, undef, $flags); } @@ -214,6 +216,8 @@ sub init_pipeline { Arg[2] : String $url. The location of the database Arg[3] : Arrayref $args. Extra arguments given to the script Arg[4] : String $test_name (optional). The name of the test + Arg[5] : (optional) Hashref $flags. Flags given to the text framework. Currently the only + key that is understood is "expect_failure" to reverse the expectation of the test. Description : Generic method that can run any eHive script and check its return status Returntype : None Exceptions : TAP-style @@ -223,12 +227,19 @@ sub init_pipeline { =cut sub _test_ehive_script { - my ($script_name, $url, $args, $test_name) = @_; + my ($script_name, $url, $args, $test_name, $flags) = @_; $args ||= []; + $flags ||= {}; my @ext_args = ( defined($url) ? (-url => $url) : (), @$args ); - $test_name ||= 'Can run '.$script_name.(@ext_args ? ' with the following cmdline options: '.join(' ', @ext_args) : ''); - ok(!system($ENV{'EHIVE_ROOT_DIR'}.'/scripts/'.$script_name.'.pl', @ext_args), $test_name); + my $rc = system($ENV{'EHIVE_ROOT_DIR'}.'/scripts/'.$script_name.'.pl', @ext_args); + if ($flags->{expect_failure}) { + $test_name ||= $script_name.' fails'.(@ext_args ? ' with the command-line options: '.join(' ', @ext_args) : ''); + ok($rc, $test_name); + } else { + $test_name ||= 'Can run '.$script_name.(@ext_args ? ' with the command-line options: '.join(' ', @ext_args) : ''); + is($rc, 0, $test_name); + } } @@ -237,6 +248,8 @@ sub _test_ehive_script { Arg[1] : String $url. The location of the database Arg[2] : Arrayref $args. Extra arguments given to runWorker Arg[3] : String $test_name (optional). The name of the test + Arg[4] : (optional) Hashref $flags. Flags given to the text framework. Currently the only + key that is understood is "expect_failure" to reverse the expectation of the test. Example : runWorker($url, [ -can_respecialize => 1 ]); Description : Run a worker on the given pipeline in the current process. The worker options have been divided in three groups: the ones affecting its specialization, @@ -269,6 +282,8 @@ sub runWorker { Arg[1] : String $url. The location of the database Arg[2] : Arrayref $args. Extra arguments given to seed_pipeline Arg[3] : String $test_name (optional). The name of the test + Arg[4] : (optional) Hashref $flags. Flags given to the text framework. Currently the only + key that is understood is "expect_failure" to reverse the expectation of the test. Example : $seed_pipeline($url, [$arg1, $arg2], 'Run seed_pipeline with two arguments'); Description : Very generic function to run seed_pipeline on the given database with the given arguments Returntype : None @@ -289,6 +304,8 @@ sub seed_pipeline { Arg[1] : String $url. The location of the database Arg[2] : Arrayref $args. Extra arguments given to beekeeper.pl Arg[3] : String $test_name (optional). The name of the test + Arg[4] : (optional) Hashref $flags. Flags given to the text framework. Currently the only + key that is understood is "expect_failure" to reverse the expectation of the test. Example : beekeeper($url, [$arg1, $arg2], 'Run beekeeper with two arguments'); Description : Very generic function to run beekeeper on the given database with the given arguments Returntype : None @@ -307,6 +324,8 @@ sub beekeeper { Arg[1] : String $url. The location of the database Arg[2] : Arrayref $args. Extra arguments given to beekeeper.pl Arg[3] : String $test_name (optional). The name of the test + Arg[4] : (optional) Hashref $flags. Flags given to the text framework. Currently the only + key that is understood is "expect_failure" to reverse the expectation of the test. Example : tweak_pipeline($url, [$arg1, $arg2], 'Run tweak_pipeline with two arguments'); Description : Very generic function to run tweak_pipeline on the given database with the given arguments Returntype : None @@ -326,6 +345,8 @@ sub tweak_pipeline { Arg[1] : String $url or undef. The location of the database Arg[2] : Arrayref $args. Extra arguments given to generate_graph.pl Arg[3] : String $test_name (optional). The name of the test + Arg[4] : (optional) Hashref $flags. Flags given to the text framework. Currently the only + key that is understood is "expect_failure" to reverse the expectation of the test. Example : generate_graph($url, [-output => 'lm_analyses.png'], 'Generate a PNG A-diagram'); Description : Very generic function to run generate_graph.pl on the given database with the given arguments Returntype : None @@ -345,6 +366,8 @@ sub generate_graph { Arg[1] : String $url. The location of the database Arg[2] : Arrayref $args. Extra arguments given to visualize_jobs.pl Arg[3] : String $test_name (optional). The name of the test + Arg[4] : (optional) Hashref $flags. Flags given to the text framework. Currently the only + key that is understood is "expect_failure" to reverse the expectation of the test. Example : visualize_jobs($url, [-output => 'lm_jobs.png', -accu_values], 'Generate a PNG J-diagram with accu values'); Description : Very generic function to run visualize_jobs.pl on the given database with the given arguments Returntype : None @@ -363,6 +386,8 @@ sub visualize_jobs { Arg[1] : String $url. The location of the database Arg[2] : Arrayref $args. Extra arguments given to peekJob.pl Arg[3] : String $test_name (optional). The name of the test + Arg[4] : (optional) Hashref $flags. Flags given to the text framework. Currently the only + key that is understood is "expect_failure" to reverse the expectation of the test. Example : peekJob($url, [-job_id => 1], 'Check params for job 1'); Description : Very generic function to run peekJob.pl on the given database with the given arguments Returntype : None @@ -382,6 +407,8 @@ sub peekJob { Arg[1] : String $url. The location of the database Arg[2] : Arrayref $args. Extra arguments given to db_cmd.pl Arg[3] : String $test_name (optional). The name of the test + Arg[4] : (optional) Hashref $flags. Flags given to the text framework. Currently the only + key that is understood is "expect_failure" to reverse the expectation of the test. Example : db_cmd($url, [-sql => 'DROP DATABASE'], 'Drop the database'); Description : Very generic function to run db_cmd.pl on the given database with the given arguments Returntype : None @@ -401,6 +428,8 @@ sub db_cmd { Arg[1] : String $url. The location of the database Arg[2] : String $sql. The SQL to run on the database Arg[3] : String $test_name (optional). The name of the test + Arg[4] : (optional) Hashref $flags. Flags given to the text framework. Currently the only + key that is understood is "expect_failure" to reverse the expectation of the test. Example : run_sql_on_db($url, 'INSERT INTO sweets (name, quantity) VALUES (3, 'Snickers')'); Description : Execute an SQL command on the given database and test its execution. This expects the command-line client to return a non-zero code in case of a failure. @@ -412,8 +441,10 @@ sub db_cmd { =cut sub run_sql_on_db { - my ($url, $sql, $test_name) = @_; - return _test_ehive_script('db_cmd', $url, [-sql => $sql], $test_name // 'Can run '.$sql); + my ($url, $sql, $test_name, $flags) = @_; + $flags ||= {}; + $test_name //= $flags->{expect_failure} ? $sql.' fails' : 'Can run '.$sql; + return _test_ehive_script('db_cmd', $url, [-sql => $sql], $test_name, $flags); } diff --git a/t/02.api/create_drop_database.t b/t/02.api/create_drop_database.t index ed463bee9..afda1d0c9 100755 --- a/t/02.api/create_drop_database.t +++ b/t/02.api/create_drop_database.t @@ -44,7 +44,7 @@ foreach my $test_url (@$ehive_test_pipeline_urls) { if ($dbc->driver eq 'sqlite') { run_sql_on_db($test_url, 'DROP DATABASE', "'rm -f' doesn't care about missing files"); } else { - is(system(@{ $dbc->to_cmd(undef, undef, undef, 'DROP DATABASE') }), 256, "Cannot drop a database that doesn't exist"); + run_sql_on_db($test_url, 'DROP DATABASE', "Can drop a database that exists", {'expect_failure' => 1}); } run_sql_on_db($test_url, 'CREATE DATABASE', 'Can create a database'); run_sql_on_db($test_url, 'CREATE DATABASE IF NOT EXISTS', 'Further CREATE DATABASE statements are ignored') unless $dbc->driver eq 'pgsql'; diff --git a/t/03.scripts/beekeeper_opts.t b/t/03.scripts/beekeeper_opts.t index 8c82c109c..a1a97d120 100755 --- a/t/03.scripts/beekeeper_opts.t +++ b/t/03.scripts/beekeeper_opts.t @@ -66,14 +66,11 @@ my $pipeline_url = get_test_url_or_die(); is($found_beekeeper_dash_run, 1, 'A beekeeper with option -run was registered in the beekeeper table'); # Check that -run -job_id with a non-existant job id fails with TASK_FAILED - # Not using beekeeper() because we expect the command to *fail* - my @bad_job_cmd = ($ENV{'EHIVE_ROOT_DIR'}.'/scripts/beekeeper.pl', -url => $hive_dba->dbc->url, '-run', -job_id => 98765); - my $rc; - my $bk_stderr = capture_stderr { - $rc = system(@bad_job_cmd); - }; - ok($rc, 'beekeeper -run -job_id 98765 exited with a non-zero return code'); - like($bk_stderr, qr/Could not fetch Job with dbID=98765/, 'beekeeper complained that the job cannot be found'); + beekeeper($hive_dba->dbc->url, + ['-run', -job_id => 98765], + 'beekeeper complained that the job cannot be found', + {'expect_failure' => 1}, + ); $beekeeper_rows = $beekeeper_nta->fetch_all(); is(scalar(@$beekeeper_rows), 3, 'After -sync, -run, and -run -job_id, there are exactly three entries in the beekeeper table'); @@ -87,16 +84,12 @@ my $pipeline_url = get_test_url_or_die(); is($found_beekeeper_bad_job, 1, 'A beekeeper with option -job_id was registered in the beekeeper table'); # Check that -loop -analyses_pattern with a non-matching pattern fails with TASK_FAILED - # Not using beekeeper() because we expect the command to *fail* - my @bad_pattern_cmd = ($ENV{'EHIVE_ROOT_DIR'}.'/scripts/beekeeper.pl', - -url => $hive_dba->dbc->url, - -analyses_pattern => 'this_matches_no_analysis', - '-loop'); - $bk_stderr = capture_stderr { - $rc = system(@bad_pattern_cmd); - }; - ok($rc, 'beekeeper -loop -analyses_pattern this_matches_no_analysis exited with a non-zero return code'); - like($bk_stderr, qr/the -analyses_pattern 'this_matches_no_analysis' did not match any Analyses/, 'beekeeper complained that no analysis could be matched'); + beekeeper( + $hive_dba->dbc->url, + ['-loop', -analyses_pattern => 'this_matches_no_analysis'], + 'beekeeper complained that no analysis could be matched', + {'expect_failure' => 1}, + ); $beekeeper_rows = $beekeeper_nta->fetch_all(); is(scalar(@$beekeeper_rows), 4, 'After 4 beekeeper commands, there are exactly 4 entries in the beekeeper table'); From 28d80c6e0e4952878924d2ca01bebdd8464169eb Mon Sep 17 00:00:00 2001 From: Matthieu Muffato Date: Wed, 17 Jun 2020 13:34:04 +0100 Subject: [PATCH 3/6] expect_failure can now be a regexp to test the error message --- modules/Bio/EnsEMBL/Hive/Utils/Test.pm | 31 +++++++++++++++++++++++++- t/03.scripts/beekeeper_opts.t | 4 ++-- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/modules/Bio/EnsEMBL/Hive/Utils/Test.pm b/modules/Bio/EnsEMBL/Hive/Utils/Test.pm index 6369d2156..039fd539c 100644 --- a/modules/Bio/EnsEMBL/Hive/Utils/Test.pm +++ b/modules/Bio/EnsEMBL/Hive/Utils/Test.pm @@ -25,6 +25,7 @@ use warnings; no warnings qw( redefine ); use Exporter; +use Capture::Tiny 'tee_stderr'; use Carp qw{croak}; use File::Spec; use File::Temp qw{tempfile}; @@ -168,6 +169,8 @@ sub standaloneJob { Arg[4] : (optional) Arrayref $tweaks. Tweaks to be applied to the database (as with the -tweak command-line option) Arg[5] : (optional) Hashref $flags. Flags given to the text framework. Currently the only key that is understood is "expect_failure" to reverse the expectation of the test. + It is primarily used as a boolean, but can be a regular expression to test the standard + error of the script. Example : init_pipeline( 'Bio::EnsEMBL::Hive::Examples::LongMult::PipeConfig::LongMultServer_conf', $server_url, @@ -218,6 +221,8 @@ sub init_pipeline { Arg[4] : String $test_name (optional). The name of the test Arg[5] : (optional) Hashref $flags. Flags given to the text framework. Currently the only key that is understood is "expect_failure" to reverse the expectation of the test. + It is primarily used as a boolean, but can be a regular expression to test the standard + error of the script. Description : Generic method that can run any eHive script and check its return status Returntype : None Exceptions : TAP-style @@ -232,10 +237,16 @@ sub _test_ehive_script { $flags ||= {}; my @ext_args = ( defined($url) ? (-url => $url) : (), @$args ); - my $rc = system($ENV{'EHIVE_ROOT_DIR'}.'/scripts/'.$script_name.'.pl', @ext_args); + my $rc; + my $stderr = tee_stderr { + $rc = system($ENV{'EHIVE_ROOT_DIR'}.'/scripts/'.$script_name.'.pl', @ext_args); + }; if ($flags->{expect_failure}) { $test_name ||= $script_name.' fails'.(@ext_args ? ' with the command-line options: '.join(' ', @ext_args) : ''); ok($rc, $test_name); + if (re::is_regexp($flags->{expect_failure})) { + like($stderr, $flags->{expect_failure}, 'error message as expected'); + } } else { $test_name ||= 'Can run '.$script_name.(@ext_args ? ' with the command-line options: '.join(' ', @ext_args) : ''); is($rc, 0, $test_name); @@ -250,6 +261,8 @@ sub _test_ehive_script { Arg[3] : String $test_name (optional). The name of the test Arg[4] : (optional) Hashref $flags. Flags given to the text framework. Currently the only key that is understood is "expect_failure" to reverse the expectation of the test. + It is primarily used as a boolean, but can be a regular expression to test the standard + error of the script. Example : runWorker($url, [ -can_respecialize => 1 ]); Description : Run a worker on the given pipeline in the current process. The worker options have been divided in three groups: the ones affecting its specialization, @@ -284,6 +297,8 @@ sub runWorker { Arg[3] : String $test_name (optional). The name of the test Arg[4] : (optional) Hashref $flags. Flags given to the text framework. Currently the only key that is understood is "expect_failure" to reverse the expectation of the test. + It is primarily used as a boolean, but can be a regular expression to test the standard + error of the script. Example : $seed_pipeline($url, [$arg1, $arg2], 'Run seed_pipeline with two arguments'); Description : Very generic function to run seed_pipeline on the given database with the given arguments Returntype : None @@ -306,6 +321,8 @@ sub seed_pipeline { Arg[3] : String $test_name (optional). The name of the test Arg[4] : (optional) Hashref $flags. Flags given to the text framework. Currently the only key that is understood is "expect_failure" to reverse the expectation of the test. + It is primarily used as a boolean, but can be a regular expression to test the standard + error of the script. Example : beekeeper($url, [$arg1, $arg2], 'Run beekeeper with two arguments'); Description : Very generic function to run beekeeper on the given database with the given arguments Returntype : None @@ -326,6 +343,8 @@ sub beekeeper { Arg[3] : String $test_name (optional). The name of the test Arg[4] : (optional) Hashref $flags. Flags given to the text framework. Currently the only key that is understood is "expect_failure" to reverse the expectation of the test. + It is primarily used as a boolean, but can be a regular expression to test the standard + error of the script. Example : tweak_pipeline($url, [$arg1, $arg2], 'Run tweak_pipeline with two arguments'); Description : Very generic function to run tweak_pipeline on the given database with the given arguments Returntype : None @@ -347,6 +366,8 @@ sub tweak_pipeline { Arg[3] : String $test_name (optional). The name of the test Arg[4] : (optional) Hashref $flags. Flags given to the text framework. Currently the only key that is understood is "expect_failure" to reverse the expectation of the test. + It is primarily used as a boolean, but can be a regular expression to test the standard + error of the script. Example : generate_graph($url, [-output => 'lm_analyses.png'], 'Generate a PNG A-diagram'); Description : Very generic function to run generate_graph.pl on the given database with the given arguments Returntype : None @@ -368,6 +389,8 @@ sub generate_graph { Arg[3] : String $test_name (optional). The name of the test Arg[4] : (optional) Hashref $flags. Flags given to the text framework. Currently the only key that is understood is "expect_failure" to reverse the expectation of the test. + It is primarily used as a boolean, but can be a regular expression to test the standard + error of the script. Example : visualize_jobs($url, [-output => 'lm_jobs.png', -accu_values], 'Generate a PNG J-diagram with accu values'); Description : Very generic function to run visualize_jobs.pl on the given database with the given arguments Returntype : None @@ -388,6 +411,8 @@ sub visualize_jobs { Arg[3] : String $test_name (optional). The name of the test Arg[4] : (optional) Hashref $flags. Flags given to the text framework. Currently the only key that is understood is "expect_failure" to reverse the expectation of the test. + It is primarily used as a boolean, but can be a regular expression to test the standard + error of the script. Example : peekJob($url, [-job_id => 1], 'Check params for job 1'); Description : Very generic function to run peekJob.pl on the given database with the given arguments Returntype : None @@ -409,6 +434,8 @@ sub peekJob { Arg[3] : String $test_name (optional). The name of the test Arg[4] : (optional) Hashref $flags. Flags given to the text framework. Currently the only key that is understood is "expect_failure" to reverse the expectation of the test. + It is primarily used as a boolean, but can be a regular expression to test the standard + error of the script. Example : db_cmd($url, [-sql => 'DROP DATABASE'], 'Drop the database'); Description : Very generic function to run db_cmd.pl on the given database with the given arguments Returntype : None @@ -430,6 +457,8 @@ sub db_cmd { Arg[3] : String $test_name (optional). The name of the test Arg[4] : (optional) Hashref $flags. Flags given to the text framework. Currently the only key that is understood is "expect_failure" to reverse the expectation of the test. + It is primarily used as a boolean, but can be a regular expression to test the standard + error of the script. Example : run_sql_on_db($url, 'INSERT INTO sweets (name, quantity) VALUES (3, 'Snickers')'); Description : Execute an SQL command on the given database and test its execution. This expects the command-line client to return a non-zero code in case of a failure. diff --git a/t/03.scripts/beekeeper_opts.t b/t/03.scripts/beekeeper_opts.t index a1a97d120..591245ae2 100755 --- a/t/03.scripts/beekeeper_opts.t +++ b/t/03.scripts/beekeeper_opts.t @@ -69,7 +69,7 @@ my $pipeline_url = get_test_url_or_die(); beekeeper($hive_dba->dbc->url, ['-run', -job_id => 98765], 'beekeeper complained that the job cannot be found', - {'expect_failure' => 1}, + {'expect_failure' => qr/Could not fetch Job with dbID=98765/}, ); $beekeeper_rows = $beekeeper_nta->fetch_all(); @@ -88,7 +88,7 @@ my $pipeline_url = get_test_url_or_die(); $hive_dba->dbc->url, ['-loop', -analyses_pattern => 'this_matches_no_analysis'], 'beekeeper complained that no analysis could be matched', - {'expect_failure' => 1}, + {'expect_failure' => qr/the -analyses_pattern 'this_matches_no_analysis' did not match any Analyses/}, ); $beekeeper_rows = $beekeeper_nta->fetch_all(); From 396a226a3bf5b4dce37d794074cbf6c19af1969a Mon Sep 17 00:00:00 2001 From: Matthieu Muffato Date: Tue, 10 Nov 2020 16:23:22 +0000 Subject: [PATCH 4/6] Drop the (partial) database if the initialisation fails --- modules/Bio/EnsEMBL/Hive/Scripts/InitPipeline.pm | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/modules/Bio/EnsEMBL/Hive/Scripts/InitPipeline.pm b/modules/Bio/EnsEMBL/Hive/Scripts/InitPipeline.pm index 94780837e..a11c586c1 100644 --- a/modules/Bio/EnsEMBL/Hive/Scripts/InitPipeline.pm +++ b/modules/Bio/EnsEMBL/Hive/Scripts/InitPipeline.pm @@ -59,7 +59,18 @@ sub init_pipeline { $hive_dba->dbc->requires_write_access(); print "> Parsing the PipeConfig file and adding objects (this may take a while).\n\n"; - $pipeconfig_object->add_objects_from_config( $pipeline ); + eval { + $pipeconfig_object->add_objects_from_config( $pipeline ); + }; + if ($@) { + print "\n> Error: $@\n"; + print "> Dropping the database due to errors.\n\n"; + my $drop_cmd = $pipeconfig_object->db_cmd('DROP DATABASE IF EXISTS'); + if (system($drop_cmd)) { + die "Can't drop the database ".$pipeconfig_object->pipeline_url(); + } + exit 1; + } if($tweaks and @$tweaks) { print "> Applying tweaks.\n\n"; From 124efb4b0ad10abff4d017018f4cc2b4c6b416da Mon Sep 17 00:00:00 2001 From: Matthieu Muffato Date: Sun, 29 Nov 2020 12:48:48 +0000 Subject: [PATCH 5/6] Extended the test to check that the database is dropped if an error occurs --- t/10.pipeconfig/analysis_heir.t | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/t/10.pipeconfig/analysis_heir.t b/t/10.pipeconfig/analysis_heir.t index 79ab9c190..50fa53939 100755 --- a/t/10.pipeconfig/analysis_heir.t +++ b/t/10.pipeconfig/analysis_heir.t @@ -18,11 +18,12 @@ use strict; use warnings; +use Test::Exception; use Test::More; use Data::Dumper; use Capture::Tiny 'capture_stderr'; -use Bio::EnsEMBL::Hive::Utils::Test qw(init_pipeline runWorker get_test_url_or_die); +use Bio::EnsEMBL::Hive::Utils::Test qw(init_pipeline runWorker db_cmd get_test_url_or_die); my $expected_error_pattern = qq{WARNING: Could not find a local analysis named 'oops_i_am_missing' \Q(dataflow from analysis 'first')}; @@ -50,5 +51,24 @@ my $gc_init_stderr = capture_stderr { unlike($gc_init_stderr, qr/WARNING/, 'no warning from pipeline without missing analysis'); +subtest 'Drop on error' => sub { + + # To ensure we start with the database being absent + db_cmd($pipeline_url, [-sql => 'DROP DATABASE IF EXISTS']); + + # Will fail because PERL5LIB hasn't been updated to include the local directory + init_pipeline('TestPipeConfig::MissingAnalysis_conf', $pipeline_url, [], [], + { + 'expect_failure' => qr/Can't locate TestPipeConfig\/MissingAnalysis_conf.pm in \@INC/, + } + ); + + my $dbc = Bio::EnsEMBL::Hive::DBSQL::DBConnection->new(-url => $pipeline_url); + throws_ok { + $dbc->connect; + } qr/Could not connect to database.*DBI connect\(.*\) failed/s, q{The database doesn't exist}; + +}; + done_testing(); From c1bee2ac6bc28985772bbec6d4be3b209448a471 Mon Sep 17 00:00:00 2001 From: Matthieu Muffato Date: Sun, 29 Nov 2020 13:54:03 +0000 Subject: [PATCH 6/6] sqlite works differently: you can actually connect to a non-existing database It will be created automatically --- t/10.pipeconfig/analysis_heir.t | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/t/10.pipeconfig/analysis_heir.t b/t/10.pipeconfig/analysis_heir.t index 50fa53939..91a882b70 100755 --- a/t/10.pipeconfig/analysis_heir.t +++ b/t/10.pipeconfig/analysis_heir.t @@ -64,9 +64,13 @@ subtest 'Drop on error' => sub { ); my $dbc = Bio::EnsEMBL::Hive::DBSQL::DBConnection->new(-url => $pipeline_url); - throws_ok { - $dbc->connect; - } qr/Could not connect to database.*DBI connect\(.*\) failed/s, q{The database doesn't exist}; + if ($dbc->driver eq 'sqlite') { + ok(!-e $dbc->dbname, $dbc->dbname . q{ doesn't exist}) + } else { + throws_ok { + $dbc->connect; + } qr/Could not connect to database.*DBI connect\(.*\) failed/s, q{The database doesn't exist}; + } };