From 9836ec090da53c4282c4ff8f1fdcafaca77d44c8 Mon Sep 17 00:00:00 2001
From: Marcelo Boeira <marcelo@boeira.me>
Date: Tue, 22 Aug 2023 14:09:00 +0200
Subject: [PATCH] fix(gitlab): Prevent considering non-head pipelines skipped
 by default

By fixing nil pointer reference in 3428 we unveiled yet another bug now that the code is able to run further:

When an MR with no HeadPipeline exists its commits are set to skipped by default and thus create some mess when running "PullMergeable" (rendering the MR useless)

The bug was caught by the tests, however, in an attempt to make the build pass and unblock we create a patch to ignore that test.

In order to prevent further problems, this MR:

* Updates the test stack to run all plan/apply commands in the context
  of "HeadLess" Pipelines
* Fixes the default of skipped pipeline to false (as it is better to
  assume it is NOT skipped since that prevents the merge in most cases)
* Make all integratiion tests pass

References:

* Original MR: https://github.com/runatlantis/atlantis/pull/3428
* Tests Patch MR : https://github.com/runatlantis/atlantis/pull/3653
---
 server/events/vcs/gitlab_client.go      |  2 +-
 server/events/vcs/gitlab_client_test.go | 25 +++++++++++++++++++++++--
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/server/events/vcs/gitlab_client.go b/server/events/vcs/gitlab_client.go
index f96d964fda..030629c8b8 100644
--- a/server/events/vcs/gitlab_client.go
+++ b/server/events/vcs/gitlab_client.go
@@ -283,7 +283,7 @@ func (g *GitlabClient) PullIsMergeable(repo models.Repo, pull models.PullRequest
 	// Prevent nil pointer error when mr.HeadPipeline is empty
 	// See: https://github.com/runatlantis/atlantis/issues/1852
 	commit := pull.HeadCommit
-	isPipelineSkipped := true
+	isPipelineSkipped := false
 	if mr.HeadPipeline != nil {
 		commit = mr.HeadPipeline.SHA
 		isPipelineSkipped = mr.HeadPipeline.Status == "skipped"
diff --git a/server/events/vcs/gitlab_client_test.go b/server/events/vcs/gitlab_client_test.go
index 5d4e6a701b..bca29ab26c 100644
--- a/server/events/vcs/gitlab_client_test.go
+++ b/server/events/vcs/gitlab_client_test.go
@@ -367,6 +367,27 @@ func TestGitlabClient_PullIsMergeable(t *testing.T) {
 			defaultMr,
 			true,
 		},
+		{
+			fmt.Sprintf("%s/apply: resource/default", vcsStatusName),
+			models.FailedCommitStatus,
+			gitlabServerVersions,
+			noHeadPipelineMR,
+			true,
+		},
+		{
+			fmt.Sprintf("%s/apply", vcsStatusName),
+			models.FailedCommitStatus,
+			gitlabServerVersions,
+			noHeadPipelineMR,
+			true,
+		},
+		{
+			fmt.Sprintf("%s/plan: resource/default", vcsStatusName),
+			models.FailedCommitStatus,
+			gitlabServerVersions,
+			noHeadPipelineMR,
+			false,
+		},
 		{
 			fmt.Sprintf("%s/plan", vcsStatusName),
 			models.PendingCommitStatus,
@@ -376,10 +397,10 @@ func TestGitlabClient_PullIsMergeable(t *testing.T) {
 		},
 		{
 			fmt.Sprintf("%s/plan", vcsStatusName),
-			models.FailedCommitStatus,
+			models.SuccessCommitStatus,
 			gitlabServerVersions,
 			noHeadPipelineMR,
-			false,
+			true,
 		},
 	}
 	for _, serverVersion := range gitlabServerVersions {