Fix #92: do todo pass/fail correctly, bump version

Changes

@@ -2,6 +2,8 @@ Revision history for Perl module TestRail::API
 
 0.035 2016-04-17 TEODESIAN
     - Fix testrail-report and testrail-cases broken binary arg passing
+    - Fix issue where TODO PASS was reported in tests with TODO FAILs.
+    - Fix issue where TODO FAILED steps were reported as TODO PASS.
 
 0.034 2016-02-18 TEODESIAN
     - Use Capture::Tiny rahter than IO::CaptureOutput in unit tests

dist.ini

@@ -1,6 +1,6 @@
 name = TestRail-API
 main_module = lib/TestRail/API.pm
-version = 0.034
+version = 0.035
 author = George S. Baugh <teodesian@cpan.org>
 license = Perl_5
 copyright_holder = George S. Baugh

lib/Test/Rail/Parser.pm

@@ -418,29 +418,37 @@ sub testCallback {
     my $todo_reason;
     #Setup args to pass to function
     my $status = $self->{'tr_opts'}->{'not_ok'}->{'id'};
+    my $status_name = 'NOT OK';
     if ($test->is_actual_ok()) {
         $status = $self->{'tr_opts'}->{'ok'}->{'id'};
+        $status_name = 'OK';
         if ($test->has_skip()) {
             $status = $self->{'tr_opts'}->{'skip'}->{'id'};
+            $status_name = 'SKIP';
             $test_name =~ s/^(ok|not ok)\s[0-9]*\s//g;
             $test_name =~ s/^# skip //gi;
+            print "# '$test_name'\n";
         }
         if ($test->has_todo()) {
             $status = $self->{'tr_opts'}->{'todo_pass'}->{'id'};
-            $test_name =~ s/^(ok|not ok)\s[0-9]*\s//g;
-            $test_name =~ s/(^# todo & skip )//gi; #handle todo_skip
-            $test_name =~ s/ # todo\s(.*)$//gi;
-            $todo_reason = $1;
+            $status_name = 'TODO PASS';
+            $test_name   =~ s/^(ok|not ok)\s[0-9]*\s//g;
+            $test_name   =~ s/^# todo & skip //gi; #handle todo_skip
+            $test_name   =~ s/# todo\s(.*)$//gi;
+            $todo_reason = $test->explanation();
         }
     } else {
         if ($test->has_todo()) {
-            $status = $self->{'tr_opts'}->{'todo_pass'}->{'id'};
-            $test_name =~ s/^(ok|not ok)\s[0-9]*\s//g;
-            $test_name =~ s/^# todo & skip //gi; #handle todo_skip
-            $test_name =~ s/# todo\s(.*)$//gi;
-            $todo_reason = $1;
+            $status = $self->{'tr_opts'}->{'todo_fail'}->{'id'};
+            $status_name = 'TODO FAIL';
+            $test_name   =~ s/^(ok|not ok)\s[0-9]*\s//g;
+            $test_name   =~ s/^# todo & skip //gi; #handle todo_skip
+            $test_name   =~ s/# todo\s(.*)$//gi;
+            $todo_reason = $test->explanation();
         }
     }
+    #XXX much of the above code would be unneeded if $test->description wasn't garbage
+    $test_name =~ s/\s+$//g;
 
     #If this is a TODO, set the reason in the notes
     $self->{'tr_opts'}->{'test_notes'} .= "\nTODO reason: $todo_reason\n" if $todo_reason;
@@ -455,7 +463,7 @@ sub testCallback {
         #XXX Obviously getting the 'expected' and 'actual' from the tap DIAGs would be ideal
         push(
             @{$self->{'tr_opts'}->{'result_custom_options'}->{'step_results'}},
-            TestRail::API::buildStepResults($line,"Good result","Bad Result",$status)
+            TestRail::API::buildStepResults($line,"OK",$status_name,$status)
         );
         print "# Appended step results.\n" if $self->{'tr_opts'}->{'debug'};
         return 1;
@@ -534,9 +542,11 @@ sub EOFCallback {
     my $test_name  = basename($self->{'file'});
 
     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
-    $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 TODO PASS
+    $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
 
     #Global status override

t/Test-Rail-Parser.t

@@ -10,8 +10,9 @@ use Scalar::Util qw{reftype};
 use TestRail::API;
 use Test::LWP::UserAgent::TestRailMock;
 use Test::Rail::Parser;
-use Test::More 'tests' => 103;
+use Test::More 'tests' => 111;
 use Test::Fatal qw{exception};
+use Test::Deep qw{cmp_deeply};
 
 #Same song and dance as in TestRail-API.t
 my $apiurl = $ENV{'TESTRAIL_API_URL'};
@@ -330,6 +331,32 @@ if (!$res) {
     is($tap->{'global_status'},8, "Test global result is TODO PASS on todo pass test");
 }
 
+undef $tap;
+$opts->{'source'} = 't/todo_pass_and_fail.test';
+$opts->{'step_results'} = 'step_results';
+$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'},1,"Errors encountered uploading case results for case that does not exist in TestRail");
+    is($tap->{'global_status'},7, "Test global result is TODO FAIL on todo pass & fail test");
+
+    my @desired_statuses = qw{1 8 7};
+    my @got_statuses = map {$_->{'status_id'}} @{$tap->{'tr_opts'}->{'result_custom_options'}->{'step_results'}};
+    my @desired_expected = ('OK', 'OK', 'OK');
+    my @got_expected = map {$_->{'expected'}} @{$tap->{'tr_opts'}->{'result_custom_options'}->{'step_results'}};
+    my @desired_actual = ('OK', 'TODO PASS', 'TODO FAIL');
+    my @got_actual = map {$_->{'actual'}} @{$tap->{'tr_opts'}->{'result_custom_options'}->{'step_results'}};
+    cmp_deeply(\@got_expected,\@desired_expected,"Expected status names look OK");
+    cmp_deeply(\@got_actual,\@desired_actual,"Actual status names look OK");
+    cmp_deeply(\@got_statuses,\@desired_statuses,"Step result status codes set correctly");
+
+    like($tap->{'tr_opts'}->{'test_notes'},qr/ez duz it/i,"TODO reason captured in test notes");
+}
+undef $opts->{'step_results'};
+
 undef $tap;
 #Check bad plan w/ todo pass logic
 $fcontents = "

t/TestRail-Utils-Find.t

@@ -105,7 +105,7 @@ $opts->{'extension'} = '.test';
 is(scalar(grep {$_ eq 'skipall.test'} @tests),0,"Tests in tree are not returned in no-match mode");
 is(scalar(grep {$_ eq 'NOT SO SEARED AFTER ALL'} @tests),0,"Tests not in tree that do exist are not returned in no-match mode");
 is(scalar(grep {$_ eq $FindBin::Bin.'/faker.test'} @tests),1,"Orphan Tests in tree ARE returned in no-match mode");
-is(scalar(@tests),6,"Correct number of non-existant cases shown (no-match, names-only)");
+is(scalar(@tests),7,"Correct number of non-existant cases shown (no-match, names-only)");
 
 $opts->{'configs'} = ['testPlatform1'];
 isnt(exception { TestRail::Utils::Find::getTests($opts,$tr) } , undef,"Correct number of non-existant cases shown (no-match, names-only)");
@@ -115,13 +115,13 @@ delete $opts->{'names-only'};
 @tests = TestRail::Utils::Find::findTests($opts,@$cases);
 my @filtered_tests = grep {defined $_} map {$_->{'full_title'}} @tests;
 is(scalar(@filtered_tests),0,"Full titles not returned in no-match mode");
-is(scalar(@tests),6,"Correct number of nonexistant cases shown in no-match mode");
+is(scalar(@tests),7,"Correct number of nonexistant cases shown in no-match mode");
 
 delete $opts->{'no-recurse'};
 $opts->{'names-only'} = 1;
 ($cases) = TestRail::Utils::Find::getTests($opts,$tr);
 @tests = TestRail::Utils::Find::findTests($opts,@$cases);
-is(scalar(@tests),10,"Correct number of non-existant cases shown (no-match, names-only, recurse)");
+is(scalar(@tests),11,"Correct number of non-existant cases shown (no-match, names-only, recurse)");
 
 #mutual excl
 $opts->{'match'} = $FindBin::Bin;
@@ -195,7 +195,7 @@ is($output->{'update'},undef,'No update cases returned');
 
 delete $opts->{'no-missing'};
 $output = TestRail::Utils::Find::findCases($opts,@$cases);
-is(scalar(@{$output->{'missing'}}),10,'Correct number of missing cases returned');
+is(scalar(@{$output->{'missing'}}),11,'Correct number of missing cases returned');
 is($output->{'orphans'},undef,'No orphan cases returned');
 is($output->{'update'},undef,'No update cases returned');
 
@@ -218,7 +218,7 @@ is($output->{'update'}->[0]->{'title'},'fake.test',"Correct update case return")
 delete $opts->{'no-missing'};
 $opts->{'orphans'} = 1;
 $output = TestRail::Utils::Find::findCases($opts,@$cases);
-is(scalar(@{$output->{'missing'}}),10,'Correct number of missing cases returned');
+is(scalar(@{$output->{'missing'}}),11,'Correct number of missing cases returned');
 is(scalar(@{$output->{'orphans'}}),1,'1 orphan case returned');
 is(scalar(@{$output->{'update'}}),1,'1 update case returned');
 

t/todo_pass.test

@@ -1,7 +1,6 @@
-use Test::More 'tests' => 3;
+use Test::More 'tests' => 2;
 pass("yay!");
 TODO: {
     local $TODO = 'Ez duz it';
     ok(1,'ruh roh');
-    ok(0,'diddley');
 }

t/todo_pass_and_fail.test

@@ -0,0 +1,7 @@
+use Test::More 'tests' => 3;
+pass("yay!");
+TODO: {
+    local $TODO = 'Ez duz it';
+    ok(1,'ruh roh');
+    ok(0,'diddley');
+}