Fix #137: Make flapping tests into failures, as they should be

Allow user overrides for clearly non-remediatable failures

Changes

@@ -3,6 +3,8 @@ Revision history for Perl module TestRail::API
 0.041 2017-06-XX TEODESIAN
     - Fix MCE usage issue with confusion based on array -> hash inputs in TestRail::Utils::Find
     - Fix issue where Test plans were not recorded in the raw output of a case.
+    - Add ability to specify a custom status for failures which emit no test plan
+    - Change tests which emit a plan but no assertions into failures.
 
 0.040 2017-05-24 TEODESIAN
     - Fix performance issue in TestRail::Utils::Find::FindTests

lib/App/Prove/Plugin/TestRail.pm

@@ -41,6 +41,7 @@ If \$HOME/.testrailrc exists, it will be parsed for any of these values in a new
     autoclose=0
     encoding=UTF-8
     configuration_group=Operating Systems
+    test_bad_status=blocked
 
 Note that passing configurations as filters for runs inside of plans are separated by colons.
 
@@ -123,6 +124,7 @@ sub load {
     $ENV{'TESTRAIL_AUTOCLOSE'} = $params->{autoclose};
     $ENV{'TESTRAIL_ENCODING'}  = $params->{encoding};
     $ENV{'TESTRIAL_CGROUP'}    = $params->{'configuration_group'};
+    $ENV{'TESTRAIL_TBAD'}      = $params->{'test_bad_status'};
     return $class;
 }
 

lib/Test/Rail/Harness.pm

@@ -51,12 +51,13 @@ sub make_parser {
     $args->{'run'}      = $ENV{'TESTRAIL_RUN'};
     $args->{'plan'}     = $ENV{'TESTRAIL_PLAN'};
     @configs = split(/:/,$ENV{'TESTRAIL_CONFIGS'}) if $ENV{'TESTRAIL_CONFIGS'};
-    $args->{'configs'}        = \@configs if scalar(@configs);
-    $args->{'result_options'} = {'version' => $ENV{'TESTRAIL_VERSION'}} if $ENV{'TESTRAIL_VERSION'};
-    $args->{'step_results'}   = $ENV{'TESTRAIL_STEPS'};
-    $args->{'testsuite_id'}   = $ENV{'TESTRAIL_SPAWN'};
-    $args->{'testsuite'}      = $ENV{'TESTRAIL_TESTSUITE'};
-    $args->{'config_group'}   = $ENV{'TESTRAIL_CGROUP'};
+    $args->{'configs'}         = \@configs if scalar(@configs);
+    $args->{'result_options'}  = {'version' => $ENV{'TESTRAIL_VERSION'}} if $ENV{'TESTRAIL_VERSION'};
+    $args->{'step_results'}    = $ENV{'TESTRAIL_STEPS'};
+    $args->{'testsuite_id'}    = $ENV{'TESTRAIL_SPAWN'};
+    $args->{'testsuite'}       = $ENV{'TESTRAIL_TESTSUITE'};
+    $args->{'config_group'}    = $ENV{'TESTRAIL_CGROUP'};
+    $args->{'test_bad_status'} = $ENV{'TESTRAIL_TBAD'};
 
     @sections = split(/:/,$ENV{'TESTRAIL_SECTIONS'}) if $ENV{'TESTRAIL_SECTIONS'};
     $args->{'sections'}  = \@sections if scalar(@sections);

lib/Test/Rail/Parser.pm

@@ -74,10 +74,12 @@ Get the TAP Parser ready to talk to TestRail, and register a bunch of callbacks
 
 =item B<sections> - ARRAYREF (optional): Restrict a spawned run to cases in these particular sections.
 
-=item B<autoclose> - BOOLEAN (optional): If no cases in the run/plan are marked 'untested' or 'retest', go ahead and close the run.  Default false.
+=item B<autoclose> - BOOLEAN (optional): If no cases in the run/plan are marked 'Untested' or 'Retest', go ahead and close the run.  Default false.
 
 =item B<encoding> - STRING (optional): Character encoding of TAP to be parsed and the various inputs parameters for the parser.  Defaults to UTF-8, see L<Encode::Supported> for a list of supported encodings.
 
+=item B<test_bad_status> - STRING (optional): 'internal' name of whatever status you want to mark compile failures & no plan + no assertion tests.
+
 =back
 
 =back
@@ -88,14 +90,16 @@ This module also attempts to calculate the elapsed time to run each test if it i
 
 The constructor will terminate if the statuses 'pass', 'fail', 'retest', 'skip', 'todo_pass', and 'todo_fail' are not registered as result internal names in your TestRail install.
 
+The purpose of the retest status is somewhat special, as there is no way to set a test back to 'untested' in TestRail, and we use this to allow automation to pick back up if
+something needs re-work for whatever reason.
+
 The global status of the case will be set according to the following rules:
 
     1. If there are no issues whatsoever besides TODO failing tests & skips, mark as PASS
     2. If there are any non-skipped or TODOed fails OR a bad plan (extra/missing tests), mark as FAIL
     3. If there are only SKIPs (e.g. plan => skip_all), mark as SKIP
     4. If the only issues with the test are TODO tests that pass, mark as TODO PASS (to denote these TODOs for removal).
-    5. If no tests are run at all, mark as 'retest'.  This is making the assumption that such failures are due to test environment being setup improperly;
-       which can be remediated and retested.
+    5. If no tests are run at all, and no plan made (such as a compile failure), the cases will be marked as failures unless you provide a test_bad status name in your testrailrc.
 
 Step results will always be whatever status is relevant to the particular step.
 
@@ -154,7 +158,8 @@ sub new {
         'config_group' => delete $opts->{'config_group'},
         #Stubs for extension by subclassers
         'result_options'        => delete $opts->{'result_options'},
-        'result_custom_options' => delete $opts->{'result_custom_options'}
+        'result_custom_options' => delete $opts->{'result_custom_options'},
+        'test_bad_status'       => delete $opts->{'test_bad_status'},
     };
 
     confess("plan passed, but no run passed!") if !$tropts->{'run'} && $tropts->{'plan'};
@@ -187,12 +192,15 @@ sub new {
     my @todof  = grep {$_->{'name'} eq 'todo_fail'} @{$tropts->{'statuses'}};
     my @todop  = grep {$_->{'name'} eq 'todo_pass'} @{$tropts->{'statuses'}};
     my @retest = grep {$_->{'name'} eq 'retest'} @{$tropts->{'statuses'}};
+    my @tbad;
+    @tbad   = grep {$_->{'name'} eq $tropts->{test_bad_status} } @{$tropts->{'statuses'}} if $tropts->{test_bad_status};
     confess("No status with internal name 'passed' in TestRail!") unless scalar(@ok);
     confess("No status with internal name 'failed' in TestRail!") unless scalar(@not_ok);
     confess("No status with internal name 'skip' in TestRail!") unless scalar(@skip);
     confess("No status with internal name 'todo_fail' in TestRail!") unless scalar(@todof);
     confess("No status with internal name 'todo_pass' in TestRail!") unless scalar(@todop);
     confess("No status with internal name 'retest' in TestRail!") unless scalar(@retest);
+    confess("No status with internal name '$tropts->{test_bad_status}' in TestRail!") unless scalar(@tbad) || !$tropts->{test_bad_status};
     #Map in all the statuses
     foreach my $status (@{$tropts->{'statuses'}}) {
         $tropts->{$status->{'name'}} = $status;
@@ -527,7 +535,9 @@ sub EOFCallback {
     my $status = $self->{'tr_opts'}->{'ok'}->{'id'};
     my $todo_failed = $self->todo() - $self->todo_passed();
     $status = $self->{'tr_opts'}->{'not_ok'}->{'id'}    if $self->has_problems();
-    $status = $self->{'tr_opts'}->{'retest'}->{'id'}    if !$self->tests_run(); #No tests were run, env fail
+    if (!$self->tests_run() && !$self->is_good_plan() && $self->{'tr_opts'}->{test_bad_status}) { #No tests were run, no plan, code is probably bad so allow custom marking
+        $status = $self->{'tr_opts'}->{$self->{'tr_opts'}->{test_bad_status}}->{'id'};
+    }
     $status = $self->{'tr_opts'}->{'todo_pass'}->{'id'} if $self->todo_passed() && !$self->failed() && $self->is_good_plan(); #If no fails, but a TODO pass, mark as TODOP
     $status = $self->{'tr_opts'}->{'todo_fail'}->{'id'} if $todo_failed && !$self->failed() && $self->is_good_plan(); #If no fails, but a TODO fail, prefer TODOF to TODOP
     $status = $self->{'tr_opts'}->{'skip'}->{'id'}      if $self->skip_all(); #Skip all, whee

t/App-Prove-Plugin-Testrail.t

@@ -6,7 +6,7 @@ use warnings;
 use FindBin;
 use lib "$FindBin::Bin/lib";
 
-use Test::More 'tests' => 8;
+use Test::More 'tests' => 9;
 use Test::Fatal;
 use Capture::Tiny qw{capture};
 use App::Prove;
@@ -49,6 +49,12 @@ $prove->process_args("-PTestRail=apiurl=http://some.testrail.install/,user=someU
 
 is (exception { capture { $prove->run() } },undef,"Running TR parser with autoclose works correctly");
 
+$prove = App::Prove->new();
+$prove->process_args("-PTestRail=apiurl=http://some.testrail.install/,user=someUser,password=somePassword,project=TestProject,plan=FinalPlan,run=FinalRun,configs=testConfig,version=0.014,test_bad_status=blocked,config_group=Operating Systems",'t/fake.test');
+
+is (exception { capture { $prove->run() } },undef,"Running TR parser with test_bad_status & config_group throws no exceptions/warnings");
+
+
 #Test multi-job upload shizz
 #Note that I don't care if it even uploads, just that it *would have* done so correctly.
 $prove = App::Prove->new();

t/Test-Rail-Parser.t

@@ -10,7 +10,7 @@ use Scalar::Util qw{reftype};
 use TestRail::API;
 use Test::LWP::UserAgent::TestRailMock;
 use Test::Rail::Parser;
-use Test::More 'tests' => 119;
+use Test::More 'tests' => 124;
 use Test::Fatal qw{exception};
 use Test::Deep qw{cmp_deeply};
 use Capture::Tiny qw{capture};
@@ -180,7 +180,7 @@ isa_ok($tap,"Test::Rail::Parser");
 if (!$res) {
     $tap->run();
     is($tap->{'errors'},0,"No errors encountered uploading case results");
-    like( $tap->{raw_output}, qr/cause I can/i, "SKIP_ALL reason recorded"); diag $tap->{raw_output};
+    like( $tap->{raw_output}, qr/cause I can/i, "SKIP_ALL reason recorded");
     is($tap->{'global_status'},6, "Test global result is SKIP on skip all");
 }
 
@@ -292,7 +292,7 @@ isa_ok($tap,"Test::Rail::Parser");
 if (!$res) {
     $tap->run();
     is($tap->{'errors'},0,"No errors encountered uploading case results");
-    is($tap->{'global_status'},4, "Test global result is RETEST on env fail");
+    is($tap->{'global_status'},5, "Test global result is FAIL by default on env fail");
 }
 
 undef $tap;
@@ -388,10 +388,25 @@ isa_ok($tap,"Test::Rail::Parser");
 if (!$res) {
     $tap->run();
     is($tap->{'errors'},0,"No errors encountered uploading case results");
-    is($tap->{'global_status'},4, "Test global result is retest when insta-bombout occurs");
+    is($tap->{'global_status'},5, "Test global result is FAILURE when insta-bombout occurs");
     my $srs = $tap->{'tr_opts'}->{'result_custom_options'}->{'step_results'};
     is($srs->[-1]->{'content'},"Bad Plan.","Bad plan noted in step results");
 }
+
+$opts->{test_bad_status} = 'bogus_status';
+$res = exception { $tap = Test::Rail::Parser->new($opts) };
+like($res,qr/bogus_status/,"TR Parser explodes on instantiation w bogus status");
+
+$opts->{test_bad_status} = 'blocked';
+$res = exception { $tap = Test::Rail::Parser->new($opts) };
+is($res,undef,"TR Parser doesn't explode on instantiation");
+isa_ok($tap,"Test::Rail::Parser");
+
+if (!$res) {
+    $tap->run();
+    is($tap->{'errors'},0,"No errors encountered uploading case results");
+    is($tap->{'global_status'},2, "Test global result is BLOCKED when insta-bombout occurs & custom status set");
+}
 undef $opts->{'step_results'};
 
 #Check unplanned tests