Test init_pipeline outside of the test process and drop the (partial) database if the initialisation fails

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";

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};
@@ -166,6 +167,10 @@ 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.
+                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,
@@ -183,7 +188,7 @@ sub standaloneJob {
 =cut
 
 sub init_pipeline {
-    my ($file_or_module, $url, $options, $tweaks) = @_;
+    my ($file_or_module, $url, $options, $tweaks, $flags) = @_;
 
     $options ||= [];
 
@@ -199,20 +204,11 @@ sub init_pipeline {
         $url = (splice(@$options, $url_flag_index, 2))[1];
     }
 
-    local @ARGV = @$options;
-    unshift @ARGV, (-pipeline_url => $url, -hive_force_init => 1);
+    my @args = ($file_or_module, -pipeline_url => $url, -hive_force_init => 1);
+    push @args, @$options;
+    push @args, map {-tweak => $_} @$tweaks if $tweaks;
 
-    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)));
+    return _test_ehive_script('init_pipeline', undef, \@args, undef, $flags);
 }
 
 
@@ -223,6 +219,10 @@ 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.
+                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,12 +232,25 @@ 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;
+    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);
+    }
 }
 
 
@@ -246,6 +259,10 @@ 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.
+                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,
@@ -278,6 +295,10 @@ 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.
+                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
@@ -298,6 +319,10 @@ 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.
+                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
@@ -316,6 +341,10 @@ 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.
+                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
@@ -335,6 +364,10 @@ 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.
+                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
@@ -354,6 +387,10 @@ 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.
+                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
@@ -372,6 +409,10 @@ 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.
+                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
@@ -391,6 +432,10 @@ 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.
+                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
@@ -410,6 +455,10 @@ 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.
+                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.
@@ -421,8 +470,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);
 }
 
 

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';

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);
 

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,

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);
     }
 
@@ -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' => qr/Could not fetch Job with dbID=98765/},
+    );
 
     $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' => qr/the -analyses_pattern 'this_matches_no_analysis' did not match any Analyses/},
+    );
 
     $beekeeper_rows = $beekeeper_nta->fetch_all();
     is(scalar(@$beekeeper_rows), 4, 'After 4 beekeeper commands, there are exactly 4 entries in the beekeeper table');

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;