Fix #57: This apparently never worked quite right

.gitignore

@@ -1,4 +1,5 @@
 *.swp
+*.swo
 Makefile.old
 cover/
 .build/

Changes

@@ -2,6 +2,7 @@ Revision history for Perl module TestRail::API
 
 0.028 2015-06-15 TEODESIAN
     - Hotfix: forgot to include a module in the prove plugin.  How did this pass compile.t? A mystery.
+    - Fix an issue where testrail-report incorrectly identified (or failed to identify) the file tested.
 
 0.027 2015-06-14 TEODESIAN
     - If no tests are run (environment fail), set test status to 'retest' on non case-per-ok tests

bin/testrail-report

@@ -94,6 +94,9 @@ Your TestRail install must have 3 custom statuses with the internal
 names 'skip', 'todo_pass', and 'todo_fail', to represent those
 states which TAP can have.
 
+Also, be sure your tests don't output non-TAP (unknown) lines ending in dots (.)
+This will cause the preceding characters to be interpreted as a test name, which may lead to unexpected results.
+
 =head2 TESTING OPTIONS:
 
     --mock don't do any real HTTP requests.
@@ -220,6 +223,12 @@ REQUIREMENTS:
   names 'skip', 'todo_pass', and 'todo_fail', to represent those
   states which TAP can have.
 
+  Also, be sure your tests don't output non-TAP (unknown) lines
+  ending in dots (.)
+  This will cause the preceding characters to be interpreted as
+  a test name, which may lead to unexpected results.
+
+
 TESTING OPTIONS:
 
     --mock don't do any real HTTP requests.
@@ -259,7 +268,6 @@ if (-e $homedir . '/.testrailrc' && (!$apiurl || !$password || !$user) ) {
     ($apiurl,$password,$user) = TestRail::Utils::parseConfig($homedir,1);
 }
 
-#XXX not even close to optimized, don't slurp in the future
 #If argument is passed use it instead of stdin
 my $file = $ARGV[0];
 die "No Such File $file" if ($file && !-e $file);
@@ -269,12 +277,10 @@ if ($file) {
     open($fh,'<',$file);
     while (<$fh>) {
         $_ = colorstrip($_); #strip prove brain damage
-        if ($_ =~ /(.*)\s\.\.$/) {
-            #File marker from default prove
-            unless ($_ =~ /^[ok|not ok] - /) {
-                push(@files,$fcontents) if $fcontents;
-                $fcontents = '';
-            }
+
+        if (TestRail::Utils::getFilenameFromTapLine($_)) {
+            push(@files,$fcontents) if $fcontents;
+            $fcontents = '';
         }
         $fcontents .= $_;
     }
@@ -292,12 +298,9 @@ if ($file) {
     }
     while (<>) {
         $_ = colorstrip($_); #strip prove brain damage
-        if ($_ =~ /(.*)\s\.\.$/) {
-            #File marker from default prove
-            unless ($_ =~ /^[ok|not ok] - /) {
-                push(@files,$fcontents) if $fcontents;
-                $fcontents = '';
-            }
+        if (TestRail::Utils::getFilenameFromTapLine($_)) {
+            push(@files,$fcontents) if $fcontents;
+            $fcontents = '';
         }
         $fcontents .= $_;
     }
@@ -351,7 +354,6 @@ $result_options = {'version' => $version} if $version;
 
 my $tap;
 foreach my $phil (@files) {
-    print "Processing $phil...\n";
     $tap = Test::Rail::Parser->new({
         'tap'          => $phil,
         'apiurl'       => $apiurl,

lib/Test/Rail/Parser.pm

@@ -12,6 +12,7 @@ use Carp qw{cluck confess};
 use POSIX qw{floor};
 
 use TestRail::API;
+use TestRail::Utils;
 use Scalar::Util qw{reftype};
 
 use File::Basename qw{basename};
@@ -297,18 +298,8 @@ sub unknownCallback {
     $self->{'raw_output'} .= "$line\n";
 
     #try to pick out the filename if we are running this on TAP in files
-
-    #old prove
-    if ($line =~ /^Running\s(.*)/) {
-        #TODO figure out which testsuite this implies
-        $self->{'file'} = $1;
-        print "# PROCESSING RESULTS FROM TEST FILE: $self->{'file'}\n";
-    }
-    #RAW tap #XXX this regex could be improved
-    if ($line =~ /(.*)\s\.\.\s*$/) {
-        $self->{'file'} = $1 unless $line =~ /^[ok|not ok] - /; #a little more careful
-    }
-    print "# $line\n" if ($line =~ /^error/i);
+    my $file = TestRail::Utils::getFilenameFromTapLine($line);
+    $self->{'file'} = $file if $file;
 }
 
 =head2 commentCallback

lib/TestRail/Utils.pm

@@ -57,6 +57,48 @@ sub parseConfig {
     return $results;
 }
 
+=head2 getFilenameFromTAPLine($line)
+
+Analyze TAP output by prove and look for filename boundaries (no other way to figure out what file is run).
+Long story short: don't end 'unknown' TAP lines with any number of dots if you don't want it interpreted as a test name.
+Apparently this is the TAP way of specifying the file that's run...which is highly inadequate.
+
+Inputs:
+
+STRING LINE - some line of TAP
+
+Returns:
+
+STRING filename of the test that output the TAP.
+
+=cut
+
+sub getFilenameFromTapLine {
+    my $orig = shift;
+
+    $orig =~ s/ *$//g; # Strip all trailing whitespace
+
+    #Special case
+    my ($is_skipall) = $orig =~ /(.*)\.+ skipped:/;
+    return $is_skipall if $is_skipall;
+
+    my @process_split = split(/ /,$orig);
+    return 0 unless scalar(@process_split);
+    my $dotty = pop @process_split; #remove the ........ (may repeat a number of times)
+    return 0 if $dotty =~ /\d/; #Apparently looking for literal dots returns numbers too. who knew?
+    chomp $dotty;
+    my $line = join(' ',@process_split);
+    
+    #IF it ends in a bunch of dots
+    #AND it isn't an ok/not ok
+    #AND it isn't a comment
+    #AND it isn't blank
+    #THEN it's a test name
+
+    return $line if ($dotty =~ /^\.+$/ && !($line =~ /^ok|not ok/) && !($line =~ /^# /) && $line);
+    return 0;
+}
+
 1;
 
 __END__

t/TestRail-Utils.t

@@ -1,7 +1,7 @@
 use strict;
 use warnings;
 
-use Test::More 'tests' => 8;
+use Test::More 'tests' => 10;
 use Test::Fatal;
 use TestRail::Utils;
 use File::Basename qw{dirname};
@@ -20,5 +20,37 @@ is($out->{apiurl},'http://hokum.bogus',"APIURL parse OK");
 is($out->{user},'zippy',"USER parse OK");
 is($out->{password}, 'happy', 'PASSWORD parse OK');
 
+#Handle both the case where we do in sequence or in paralell and mash together logs
+
+my @files;
+my $fcontents = '';
+open(my $fh,'<','t/test_multiple_files.tap') or die("couldn't open our own test files!!!");
+while (<$fh>) {
+    if (TestRail::Utils::getFilenameFromTapLine($_)) {
+        push(@files,$fcontents) if $fcontents;
+        $fcontents = '';
+    }
+    $fcontents .= $_;
+}
+close($fh);
+push(@files,$fcontents);
+diag explain \@files;
+is(scalar(@files),2,"Detects # of filenames correctly in TAP");
+
+$fcontents = '';
+@files = ();
+open($fh,'<','t/seq_multiple_files.tap') or die("couldn't open our own test files!!!");
+while (<$fh>) {
+    if (TestRail::Utils::getFilenameFromTapLine($_)) {
+        push(@files,$fcontents) if $fcontents;
+        $fcontents = '';
+    }
+    $fcontents .= $_;
+}
+close($fh);
+push(@files,$fcontents);
+diag explain \@files;
+is(scalar(@files),7,"Detects # of filenames correctly in TAP");
+
 
 #Regrettably, I have yet to find a way to print to stdin without eval, so userInput will remain untested.

t/seq_multiple_files.tap

@@ -0,0 +1,73 @@
+t/faker.test ......
+1..4
+ok 1 - Expected result OK
+not ok 2 - Unexpected result not OK
+
+#   Failed test 'Unexpected result not OK'
+#   at t/faker.test line 9.
+#          got: 'expected'
+#     expected: 'unexpected'
+ok 3 - whoop deee # TODO Making sure this test is still marked fail, not todo pass
+ok 4 # skip cuz I can
+# Looks like you failed 1 test of 4.
+Dubious, test returned 1 (wstat 256, 0x100)
+Failed 1/4 subtests
+    (less 1 skipped subtest: 2 okay)
+    (1 TODO test unexpectedly succeeded)
+t/fake.test .......
+1..2
+ok 1 - STORAGE TANKS SEARED
+# whee
+not ok 2 - NOT SO SEARED AFTER ARR
+
+#   Failed test 'NOT SO SEARED AFTER ARR'
+#   at t/fake.test line 10.
+# Looks like you failed 1 test of 2.
+Dubious, test returned 1 (wstat 256, 0x100)
+Failed 1/2 subtests
+t/notests.test ....
+1..2
+# whee
+Died at t/notests.test line 5.
+# Looks like your test exited with 255 before it could output anything.
+Dubious, test returned 255 (wstat 65280, 0xff00)
+Failed 2/2 subtests
+t/pass.test .......
+1..1
+ok 1 - yay!
+ok
+t/skipall.test .... skipped: cause I can
+t/skip.test .......
+1..3
+ok 1 # skip STORAGE TANKS SEARED
+not ok 2 # TODO & SKIP NOT SO SEARED AFTER ARR
+ok 3 - STORAGE TANKS SEARED # TODO skippity doo dah
+ok
+t/todo_pass.test ..
+1..3
+ok 1 - yay!
+ok 2 - ruh roh # TODO Ez duz it
+not ok 3 - diddley # TODO Ez duz it
+
+#   Failed (TODO) test 'diddley'
+#   at t/todo_pass.test line 6.
+ok
+
+Test Summary Report
+-------------------
+t/faker.test    (Wstat: 256 Tests: 4 Failed: 1)
+  Failed test:  2
+  TODO passed:   3
+  Non-zero exit status: 1
+t/fake.test     (Wstat: 256 Tests: 2 Failed: 1)
+  Failed test:  2
+  Non-zero exit status: 1
+t/notests.test  (Wstat: 65280 Tests: 0 Failed: 0)
+  Non-zero exit status: 255
+  Parse errors: Bad plan.  You planned 2 tests but ran 0.
+t/skip.test     (Wstat: 0 Tests: 3 Failed: 0)
+  TODO passed:   3
+t/todo_pass.test (Wstat: 0 Tests: 3 Failed: 0)
+  TODO passed:   2
+Files=7, Tests=13,  2 wallclock secs ( 0.04 usr  0.01 sys +  0.10 cusr  0.02 csys =  0.17 CPU)
+Result: FAIL

t/server_dead.t

@@ -5,7 +5,7 @@ use strict;
 use warnings;
 
 use TestRail::API;
-use Test::More 'tests' => 52;
+use Test::More 'tests' => 54;
 use Test::Fatal;
 use Class::Inspector;
 use Test::LWP::UserAgent;
@@ -69,3 +69,5 @@ is($tr->getUserByID(1),0,'getUserByID returns error');
 is($tr->getUserByName('zap'),0,'getUserByName returns error');
 is($tr->getUsers(),-500,'getUsers returns error');
 is($tr->getConfigurations(1),-500,'getConfigurations returns error');
+is($tr->closePlan(1),-500,'closePlan returns error');
+is($tr->closeRun(1),-500,'closeRun returns error');

t/test_multiple_files.tap

@@ -16,7 +16,7 @@ test.t (Wstat: 256 Tests: 2 Failed: 1)
     Files=1, Tests=2,  0 wallclock secs ( 0.02 usr  0.00 sys +  0.01 cusr  0.00 csys =  0.03 CPU)
     Result: FAIL
 
-STROGGIFY POPULATION CENTERS ..
+STROGGIFY POPULATION CENTERS ......
 ok 1 - STROGGIFY POPULATION CENTERS
 not ok 2 - STROGGIFY POPULATION CENTERS