Re-factor to more usage of TestRail::Utils (#66), make sure tests pass

Also re-factor TestRail::Utils::Lock::pickAndLockTest to return more information

bin/testrail-bulk-mark-results

@@ -123,29 +123,7 @@ my $tr = TestRail::API->new($opts->{apiurl},$opts->{user},$opts->{password},$opt
 $tr->{'browser'} = $opts->{'browser'} if $opts->{'browser'};
 $tr->{'debug'} = 0;
 
-my $project = $tr->getProjectByName($opts->{'project'});
-if (!$project) {
-    print "No such project '$opts->{project}'.\n";
-    exit 6;
-}
-
-my ($run,$plan);
-
-if ($opts->{'plan'}) {
-    $plan = $tr->getPlanByName($project->{'id'},$opts->{'plan'});
-    if (!$plan) {
-        print "No such plan '$opts->{plan}'!\n";
-        exit 1;
-    }
-    $run = $tr->getChildRunByName($plan,$opts->{'run'}, $opts->{'configs'});
-} else {
-    $run = $tr->getRunByName($project->{'id'},$opts->{'run'});
-}
-
-if (!$run) {
-    print "No such run '$opts->{run}' matching the provided configs (if any).\n";
-    exit 2;
-}
+my ($project,$plan,$run) = TestRail::Utils::getRunInformation($tr,$opts);
 
 my $user_ids;
 #Process assignedto ids

bin/testrail-lock

@@ -13,7 +13,7 @@ testrail-lock - pick an untested/retest test in TestRail, lock it, and return th
 
 It is useful to lock the test in situations where you have multiple disconnected test running processes trying to allocate resources toward testing outstanding cases so that effort is not duplicated.
 This is accomplished via setting a special locking result on a test rather than simple assignment, as detecting lock conflicts is impossible then due to a lack of assignment history.
-Results, however have a history of results set, so we use that fact to detect if a locking collision occured (race condition) and fail to return a result when another process locked during our attempt to lock.
+Results, however have a history of results set, so we use that fact to detect if a locking collision occurred (race condition) and fail to return a result when another process locked during our attempt to lock.
 
 Will respect test priority when making the choice of what test to lock.
 
@@ -47,9 +47,9 @@ All mandatory options not passed with the above switches, or in your ~/.testrail
 
 -p --plan       : desired plan name.  Required if the run passed is a child of a plan.
 
--m --match      : attempt to find filenames matching the test names in the provided dir.
+-m --match      : attempt to find filenames matching the test names in the provided directory.
 
---no-match      : attempt to find filenames that do not match test names in the provided dir.
+--no-match      : attempt to find filenames that do not match test names in the provided directory.
 
 -n --no-recurse : if match (or no-match) passed, do not recurse subdirectories.
 
@@ -126,7 +126,11 @@ $opts->{'hostname'} = hostname;
 
 TestRail::Utils::interrogateUser($opts,qw{apiurl user password project run lockname});
 
-print TestRail::Utils::pickAndLockTest($opts)."\n";
+my $ret = TestRail::Utils::pickAndLockTest($opts);
+
+exit 255 if !$ret;
+
+print $ret->{'path'}."\n";
 exit 0;
 
 __END__

bin/testrail-report

@@ -118,14 +118,6 @@ use File::HomeDir qw{my_home};
 
 print "testrail-report\n----------------------\n";
 
-use Pod::Perldoc 3.10;
-
-sub help {
-    @ARGV = ($0);
-    Pod::Perldoc->run();
-    exit 0;
-}
-
 #Main loop------------
 my %opts;
 
@@ -149,7 +141,7 @@ GetOptions(
     'help'           => \$opts{help}
 );
 
-if ($opts{help}) { help(); }
+if ($opts{help}) { TestRail::Utils::help(); }
 
 #Parse config file if we are missing api url/key or user
 my $homedir = my_home() || '.';

bin/testrail-tests

@@ -37,9 +37,9 @@ All mandatory options not passed with the above switches, or in your ~/.testrail
 
 -p --plan       : desired plan name.  Required if the run passed is a child of a plan.
 
--m --match      : attempt to find filenames matching the test names in the provided dir.
+-m --match      : attempt to find filenames matching the test names in the provided directory.
 
---no-match      : attempt to find filenames that do not match test names in the provided dir.
+--no-match      : attempt to find filenames that do not match test names in the provided directory.
 
 -n --no-recurse : if match (or no-match) passed, do not recurse subdirectories.
 
@@ -127,29 +127,7 @@ my $tr = TestRail::API->new($opts->{apiurl},$opts->{user},$opts->{password},$opt
 $tr->{'browser'} = $opts->{'browser'} if $opts->{'browser'};
 $tr->{'debug'} = 0;
 
-my $project = $tr->getProjectByName($opts->{'project'});
-if (!$project) {
-    print "No such project '$opts->{project}'.\n";
-    exit 6;
-}
-
-my ($run,$plan);
-
-if ($opts->{'plan'}) {
-    $plan = $tr->getPlanByName($project->{'id'},$opts->{'plan'});
-    if (!$plan) {
-        print "No such plan '$opts->{plan}'!\n";
-        exit 1;
-    }
-    $run = $tr->getChildRunByName($plan,$opts->{'run'}, $opts->{'configs'});
-} else {
-    $run = $tr->getRunByName($project->{'id'},$opts->{'run'});
-}
-
-if (!$run) {
-    print "No such run '$opts->{run}' matching the provided configs (if any).\n";
-    exit 2;
-}
+my ($project,$plan,$run) = TestRail::Utils::getRunInformation($tr,$opts);
 
 my ($status_ids,$user_ids);
 

dist.ini

@@ -112,6 +112,10 @@ stopwords = userInput
 stopwords = autoclose
 stopwords = closeRun
 stopwords = closePlan
+stopwords = lockTest
+stopwords = getRunInformation
+stopwords = assignedto
+stopwords = lockname
 
 [PkgVersion]
 [AutoPrereqs]

lib/Test/LWP/UserAgent/TestRailMock.pm

@@ -1242,6 +1242,34 @@ $mockObject->map_response(qr/\Q$VAR1\E$/,HTTP::Response->new($VAR2, $VAR3, $VAR4
 
 }
 
+{
+
+$VAR1 = 'index.php?/api/v2/get_tests/1&status_id=1';
+$VAR2 = '200';
+$VAR3 = 'OK';
+$VAR4 = bless( {
+                 'connection' => 'close',
+                 'x-powered-by' => 'PHP/5.5.9-1ubuntu4.5',
+                 'client-response-num' => 1,
+                 'date' => 'Tue, 23 Dec 2014 20:02:10 GMT',
+                 'client-peer' => '192.168.122.217:80',
+                 'content-length' => '276',
+                 '::std_case' => {
+                                   'client-date' => 'Client-Date',
+                                   'x-powered-by' => 'X-Powered-By',
+                                   'client-response-num' => 'Client-Response-Num',
+                                   'client-peer' => 'Client-Peer'
+                                 },
+                 'client-date' => 'Tue, 23 Dec 2014 20:02:11 GMT',
+                 'content-type' => 'application/json; charset=utf-8',
+                 'server' => 'Apache/2.4.7 (Ubuntu)'
+               }, 'HTTP::Headers' );
+$VAR5 = '[{"id":15,"case_id":8,"status_id":3,"assignedto_id":null,"run_id":22,"title":"STORAGE TANKS SEARED","type_id":6,"priority_id":4,"estimate":null,"estimate_forecast":null,"refs":null,"milestone_id":null,"custom_preconds":null,"custom_steps":null,"custom_expected":null},{"id":15,"case_id":8,"status_id":3,"assignedto_id":null,"run_id":22,"title":"NOT SO SEARED AFTER ARR"},{"id":15,"case_id":8,"status_id":3,"assignedto_id":1,"run_id":22,"title":"skipall.test"} ]';
+$mockObject->map_response(qr/\Q$VAR1\E$/,HTTP::Response->new($VAR2, $VAR3, $VAR4, $VAR5));
+
+}
+
+
 {
 
 $VAR1 = 'index.php?/api/v2/get_tests/777';

lib/TestRail/Utils.pm

@@ -137,8 +137,6 @@ sub getFilenameFromTapLine {
 =head2 getRunInformation
 
 Return the relevant project definition, plan and run definition HASHREFs for the provided options.
-Practically all the binaries need this information, so it has been subroutined out.
-
 
 Dies in the event the project/plan/run could not be found.
 

lib/TestRail/Utils/Lock.pm

@@ -34,13 +34,18 @@ testrail-lock's primary routine.
 
 =over 4
 
-=item HASHREF C<OPTIONS> - valid keys/values correspond to the longnames of arguments taken by L<testrail-lock>.
+=item HASHREF C<OPTIONS> - valid keys/values correspond to the long names of arguments taken by L<testrail-lock>.
 
 =item TestRail::API C<HANDLE> - Instance of TestRail::API, in the case where the caller already has a valid object.
 
 There is a special key, 'mock' in the HASHREF that is used for testing.
 The 'hostname' key must also be passed in the options, as it is required by lockTest, which this calls.
 
+Returns a HASHREF with the test, project, run and plan (if any) definition HASHREFs as keys.
+Also, a 'path' key will be set which has the full path to the test on disk, if match mode is passed, the case title otherwise.
+
+If the test could not be locked, 0 is returned.
+
 =back
 
 =cut
@@ -82,15 +87,25 @@ sub pickAndLockTest {
     # Filter by match options
     @$cases = TestRail::Utils::findTests($opts,@$cases);
 
-    my $title;
-    foreach my $test (@$cases) {
+    my ($title,$test);
+    while (@$cases) {
+        $test = shift @$cases;
         $title = lockTest($test,$lock_status_id,$opts->{'hostname'},$tr);
         last if $title;
     }
 
-    warn "Failed to lock case!  This probably means you don't have any cases left to lock." if !$title;
+    if (!$title) {
+        warn "Failed to lock case!  This probably means you don't have any cases left to lock.";
+        return 0;
+    }
 
-    return $title;
+    return {
+        'test'    => $test,
+        'path'    => $title,
+        'project' => $project,
+        'plan'    => $plan,
+        'run'     => $run
+    };
 }
 
 =head2 lockTest(test,lock_status_id,handle)
@@ -99,7 +114,7 @@ Lock the specified test, and return it's title (or full_title if it exists).
 
 =over 4
 
-=item HASHREF C<TEST> - Test object returned by getTest, or a similar method.
+=item HASHREF C<TEST> - Test object returned by getTests, or a similar method.
 
 =item INTEGER C<LOCK_STATUS_ID> - Status used to denote locking of test
 
@@ -107,7 +122,7 @@ Lock the specified test, and return it's title (or full_title if it exists).
 
 =back
 
-Returns undef in the event a lock could not occur, and warns & returns 0 on lock collisions.
+Returns -1 in the event a lock could not occur, and warns & returns 0 on lock collisions.
 
 =cut
 
@@ -150,7 +165,7 @@ sub lockTest {
 
     #Prefer full titles (match mode)
     return defined($test->{'full_title'}) ? $test->{'full_title'} : $test->{'title'} if $next_one;
-    return undef;
+    return -1;
 }
 
 1;

t/TestRail-Utils-Lock.t

@@ -30,19 +30,25 @@ my $tr = new TestRail::API($apiurl,$login,$pw,undef,1);
 $tr->{'debug'} = 0;
 
 $tr->{'browser'} = Test::LWP::UserAgent::TestRailMock::lockMockStep0();
-is(TestRail::Utils::Lock::pickAndLockTest($opts,$tr),undef,"Verify that no tests are locked in match mode, as they all are in a subdir, and recurse is off");
+
+my $ret = TestRail::Utils::Lock::pickAndLockTest($opts,$tr);
+is($ret,0,"Verify that no tests are locked in match mode, as they all are in a subdir, and recurse is off");
 delete $opts->{'no-recurse'};
 
-is(basename( TestRail::Utils::Lock::pickAndLockTest($opts,$tr) ), 'lockmealso.test' , "Verify the highest priority test is chosen first");
+$ret = TestRail::Utils::Lock::pickAndLockTest($opts,$tr);
+is(basename( $ret->{'path'} ), 'lockmealso.test' , "Verify the highest priority test is chosen first");
 $tr->{'browser'} = Test::LWP::UserAgent::TestRailMock::lockMockStep1();
 
-is(basename( TestRail::Utils::Lock::pickAndLockTest($opts,$tr) ), 'lockme.test'     , "Verify the highest priority test of type automated is chosen");
+$ret = TestRail::Utils::Lock::pickAndLockTest($opts,$tr);
+is(basename( $ret->{'path'} ), 'lockme.test'     , "Verify the highest priority test of type automated is chosen");
 $tr->{'browser'} = Test::LWP::UserAgent::TestRailMock::lockMockStep2();
 
-is(basename( TestRail::Utils::Lock::pickAndLockTest($opts,$tr) ), 'lockmetoo.test'  , "Verify that the highest priority test that exists in the tree is chosen");
+$ret = TestRail::Utils::Lock::pickAndLockTest($opts,$tr);
+is(basename( $ret->{'path'} ), 'lockmetoo.test'  , "Verify that the highest priority test that exists in the tree is chosen");
 $tr->{'browser'} = Test::LWP::UserAgent::TestRailMock::lockMockStep3();
 
-is(TestRail::Utils::Lock::pickAndLockTest($opts,$tr),undef,"Verify that no tests are locked, as they either are of the wrong type or do not exist in the match tree");
+$ret = TestRail::Utils::Lock::pickAndLockTest($opts,$tr);
+is($ret,0,"Verify that no tests are locked, as they either are of the wrong type or do not exist in the match tree");
 $tr->{'browser'} = Test::LWP::UserAgent::TestRailMock::lockMockStep4();
 
 #Simulate lock collision
@@ -54,17 +60,23 @@ $tr->{'browser'} = Test::LWP::UserAgent::TestRailMock::lockMockStep5();
 #Test with a second set of options, verify that no-match and type filtering works
 delete $opts->{'match'};
 $opts->{'case-types'} = ['Other'];
-is(TestRail::Utils::Lock::pickAndLockTest($opts,$tr),'sortalockme.test',"Test which is here but the other type is locked when ommitting match");
+
+$ret = TestRail::Utils::Lock::pickAndLockTest($opts,$tr);
+is($ret->{'path'},'sortalockme.test',"Test which is here but the other type is locked when ommitting match");
 $tr->{'browser'} = Test::LWP::UserAgent::TestRailMock::lockMockStep6();
 
-is(TestRail::Utils::Lock::pickAndLockTest($opts,$tr),'dontlockme_alsonothere.test',"Test which is not here but the other type is locked when omitting match");
+$ret = TestRail::Utils::Lock::pickAndLockTest($opts,$tr);
+is($ret->{'path'},'dontlockme_alsonothere.test',"Test which is not here but the other type is locked when omitting match");
 $tr->{'browser'} = Test::LWP::UserAgent::TestRailMock::lockMockStep7();
 
-is(TestRail::Utils::Lock::pickAndLockTest($opts,$tr),undef,"No tests are locked, as they are not the right type");
+$ret = TestRail::Utils::Lock::pickAndLockTest($opts,$tr);
+is($ret,0,"No tests are locked, as they are not the right type");
 
 #Make sure we only grab retest/untested
 delete $opts->{'case-types'};
-is(TestRail::Utils::Lock::pickAndLockTest($opts,$tr),'dontlockme_nothere.test',"Wrong type test which is not here gets locked after we remove all restrictions");
+$ret = TestRail::Utils::Lock::pickAndLockTest($opts,$tr);
+is($ret->{'path'},'dontlockme_nothere.test',"Wrong type test which is not here gets locked after we remove all restrictions");
 $tr->{'browser'} = Test::LWP::UserAgent::TestRailMock::lockMockStep8();
 
-is(TestRail::Utils::Lock::pickAndLockTest($opts,$tr),undef,"No tests are locked, as none are untested or retest");
+$ret = TestRail::Utils::Lock::pickAndLockTest($opts,$tr);
+is($ret,0,"No tests are locked, as none are untested or retest");

t/testrail-runs.t

@@ -8,7 +8,7 @@ my @args = ($^X,qw{bin/testrail-runs --apiurl http://testrail.local --user "test
 my $out = `@args`;
 is($? >> 8, 0, "Exit code OK looking for runs with passes");
 chomp $out;
-like($out,qr/^TestingSuite\nOtherOtherSuite\nFinalRun$/,"Gets run correctly looking for passes");
+like($out,qr/^TestingSuite\nOtherOtherSuite\nFinalRun\nlockRun$/,"Gets run correctly looking for passes");
 
 #check status filters
 @args = ($^X,qw{bin/testrail-runs --apiurl http://testrail.local --user "test@fake.fake" --password "fake" -j "TestProject" --mock --status passed});

t/testrail-tests.t

@@ -1,7 +1,7 @@
 use strict;
 use warnings;
 
-use Test::More "tests" => 32;
+use Test::More "tests" => 30;
 
 #check plan mode
 my @args = ($^X,qw{bin/testrail-tests --apiurl http://testrail.local --user "test@fake.fake" --password "fake" -j TestProject -p "GosPlan" -r "Executing the great plan" -m t --config testConfig --mock --no-recurse});
@@ -52,8 +52,6 @@ like($out,qr/skipall\.test$/,"Gets test correctly in no plan mode, recurse");
 @args = ($^X,qw{bin/testrail-tests --apiurl http://testrail.local --user "test@fake.fake" --password "fake" -j TestProject -p "GosPlan" -r "Executing the great plan" -m t --mock --config testPlatform1});
 $out = `@args`;
 isnt($? >> 8, 0, "Exit code not OK when passing invalid configs for plan");
-chomp $out;
-like($out,qr/no such run/i,"Gets test correctly in plan mode, recurse");
 
 #check assignedto filters
 @args = ($^X,qw{bin/testrail-tests --apiurl http://testrail.local --user "test@fake.fake" --password "fake" -j TestProject -p "GosPlan" -r "Executing the great plan" --mock --config "testConfig" --assignedto teodesian});
@@ -96,6 +94,3 @@ like($out,qr/encoding of arguments/i,"Help output OK");
 @args = ($^X,qw{bin/testrail-tests --no-match t/ --match t/qa --apiurl http://testrail.local --user "test@fake.fake" --password "fake" -j TestProject -r "TestingSuite" --mock});
 $out = `@args`;
 isnt($? >> 8, 0, "Exit code not OK asking for mutually exclusive match options");
-like($out,qr/mutually exclusive/i,"Death message OK");
-
-