Refactor format_git_error

lib/OpenQA/Git.pm

@@ -31,9 +31,11 @@ sub _prepare_git_command ($self, $args = undef) {
     return ('git', '-C', $dir);
 }
 
-sub _format_git_error ($result, $error_message) {
+sub _format_git_error ($self, $result, $error_message) {
+
+    my $path = $self->dir;
     if ($result->{stderr} or $result->{stdout}) {
-        $error_message .= ': ' . $result->{stdout} . $result->{stderr};
+        $error_message .= "($path): " . $result->{stdout} . $result->{stderr};
     }
     return $error_message;
 }
@@ -51,16 +53,16 @@ sub set_to_latest_master ($self, $args = undef) {
 
     if (my $update_remote = $self->config->{update_remote}) {
         my $res = run_cmd_with_log_return_error([@git, 'remote', 'update', $update_remote]);
-        return _format_git_error($res, 'Unable to fetch from origin master') unless $res->{status};
+        return $self->_format_git_error($res, 'Unable to fetch from origin master') unless $res->{status};
     }
 
     if (my $update_branch = $self->config->{update_branch}) {
         if ($self->config->{do_cleanup} eq 'yes') {
             my $res = run_cmd_with_log_return_error([@git, 'reset', '--hard', 'HEAD']);
-            return _format_git_error($res, 'Unable to reset repository to HEAD') unless $res->{status};
+            return $self->_format_git_error($res, 'Unable to reset repository to HEAD') unless $res->{status};
         }
         my $res = run_cmd_with_log_return_error([@git, 'rebase', $update_branch]);
-        return _format_git_error($res, 'Unable to reset repository to origin/master') unless $res->{status};
+        return $self->_format_git_error($res, 'Unable to reset repository to origin/master') unless $res->{status};
     }
 
     return undef;
@@ -77,29 +79,30 @@ sub commit ($self, $args = undef) {
         next unless $args->{$cmd};
         push(@files, @{$args->{$cmd}});
         my $res = run_cmd_with_log_return_error([@git, $cmd, @{$args->{$cmd}}]);
-        return _format_git_error($res, "Unable to $cmd via Git") unless $res->{status};
+        return $self->_format_git_error($res, "Unable to $cmd via Git") unless $res->{status};
     }
 
     # commit changes
     my $message = $args->{message};
     my $author = sprintf('--author=%s <%s>', $self->user->fullname, $self->user->email);
     my $res = run_cmd_with_log_return_error([@git, 'commit', '-q', '-m', $message, $author, @files]);
-    return _format_git_error($res, 'Unable to commit via Git') unless $res->{status};
+    return $self->_format_git_error($res, 'Unable to commit via Git') unless $res->{status};
 
     # push changes
     if (($self->config->{do_push} || '') eq 'yes') {
         $res = run_cmd_with_log_return_error([@git, 'push']);
-        return _format_git_error($res, 'Unable to push Git commit') unless $res->{status};
+        return $self->_format_git_error($res, 'Unable to push Git commit') unless $res->{status};
     }
 
     return undef;
 }
 
 # Moved functions from Clone.pm
 
-sub get_current_branch ($self, $path) {
-    my $r = run_cmd_with_log_return_error(['git', '-C', $path, 'branch', '--show-current']);
-    die "Error detecting current branch for '$path': $r->{stderr}" unless $r->{status};
+sub get_current_branch ($self) {#
+    my @git = $self->_prepare_git_command();
+    my $r = run_cmd_with_log_return_error([@git, 'branch', '--show-current']);
+    die "Error detecting current branch for : $r->{stderr}" unless $r->{status};
     return Mojo::Util::trim($r->{stdout});
 }
 
@@ -114,7 +117,7 @@ sub get_remote_default_branch ($self, $url) {
     return $1;
 }
 
-sub git_clone_url_to_path ($self, $url, $path) {
+sub git_clone_url_to_path ($self, $url, $path) {  
     my $r = run_cmd_with_log_return_error($self->_ssh_git_cmd(['clone', $url, $path]));
     die "Failed to clone $url into '$path': $r->{stderr}" unless $r->{status};
 }

lib/OpenQA/Task/Git/Clone.pm

@@ -22,7 +22,7 @@ sub _git_clone_all ($job, $clones) {
     my $retry_delay = {delay => 30 + int(rand(10))};
     for my $path (sort keys %$clones) {
         $path = Mojo::File->new($path)->realpath if (-e $path);    # resolve symlinks
-        my $guard = $app->minion->guard(  "git_clone_${path}_task", 2 * ONE_HOUR);
+        my $guard = $app->minion->guard("git_clone_${path}_task", 2 * ONE_HOUR);
         return $job->retry($retry_delay) unless $guard;
         push(@guards, $guard);
     }
@@ -32,7 +32,7 @@ sub _git_clone_all ($job, $clones) {
 
     # iterate clones sorted by path length to ensure that a needle dir is always cloned after the corresponding casedir
     for my $path (sort { length($a) <=> length($b) } keys %$clones) {
-        my $git = OpenQA::Git->new(app => $app);
+        my $git = OpenQA::Git->new(app => $app, dir => $path);
         my $url = $clones->{$path};
         die "Don't even think about putting '..' into '$path'." if $path =~ /\.\./;
         eval { _git_clone($git, $job, $ctx, $path, $url) };
@@ -66,7 +66,7 @@ sub _git_clone ($git, $job, $ctx, $path, $url) {
         return;
     }
 
-    my $current_branch = $git->get_current_branch($path);
+    my $current_branch = $git->get_current_branch;
     if ($requested_branch eq $current_branch) {
         # updating default branch (including checkout)
         $git->git_fetch($path, $requested_branch);

t/14-grutasks.t

@@ -687,6 +687,8 @@ subtest 'git clone' => sub {
         is $res->{state}, 'inactive', 'job set back to inactive';
     };
     subtest 'git clone fails when all retry attempts exhausted' => sub {
+        my $openqa_clone = Test::MockModule->new('OpenQA::Task::Git::Clone');
+        $openqa_clone->redefine(_git_clone => sub (@) { die "fake error\n" });
         $ENV{OPENQA_GIT_CLONE_RETRIES} = 0;
         $res = run_gru_job($t->app, 'git_clone', $clone_dirs, {priority => 10});
         is $res->{retries}, 0, 'job retries not incremented';