From d8aa0b151c5444a49ddc41b005f31086a9999258 Mon Sep 17 00:00:00 2001
From: Mark Lapierre <mlapierre@gitlab.com>
Date: Thu, 7 Jul 2022 11:47:46 +1000
Subject: [PATCH] Skip review app on quarantine only changes

---
 .gitlab/ci/qa.gitlab-ci.yml                   |  4 +--
 .gitlab/ci/review-apps/skip-qa.gitlab-ci.yml  | 13 ++++++++
 .gitlab/ci/review.gitlab-ci.yml               | 33 ++++++++++++++++++-
 .../qa/{package_and_qa_check => run_qa_check} | 12 +++----
 4 files changed, 53 insertions(+), 9 deletions(-)
 create mode 100644 .gitlab/ci/review-apps/skip-qa.gitlab-ci.yml
 rename tooling/bin/qa/{package_and_qa_check => run_qa_check} (69%)

diff --git a/.gitlab/ci/qa.gitlab-ci.yml b/.gitlab/ci/qa.gitlab-ci.yml
index 12275260c0cc..4ee080343a9d 100644
--- a/.gitlab/ci/qa.gitlab-ci.yml
+++ b/.gitlab/ci/qa.gitlab-ci.yml
@@ -97,7 +97,7 @@ populate-qa-tests-var:
     - tooling/bin/find_change_diffs ${CHANGES_DIFFS_DIR}
   script:
     - 'echo "QA_TESTS: $QA_TESTS"'
-    - exit_code=0 && tooling/bin/qa/package_and_qa_check ${CHANGES_DIFFS_DIR} || exit_code=$?
+    - exit_code=0 && tooling/bin/qa/run_qa_check ${CHANGES_DIFFS_DIR} || exit_code=$?
     - echo $exit_code
     - |
       if [ $exit_code -eq 0 ]; then
@@ -105,7 +105,7 @@ populate-qa-tests-var:
       elif [ $exit_code -eq 1 ]; then
         exit 1
       else
-        echo "Downstream jobs will not be triggered because package_and_qa_check exited with code: $exit_code"
+        echo "Downstream jobs will not be triggered because run_qa_check exited with code: $exit_code"
       fi
   # These jobs often time out, so temporarily use private runners and a long timeout: https://gitlab.com/gitlab-org/gitlab/-/issues/238563
   tags:
diff --git a/.gitlab/ci/review-apps/skip-qa.gitlab-ci.yml b/.gitlab/ci/review-apps/skip-qa.gitlab-ci.yml
new file mode 100644
index 000000000000..1305673a4d8b
--- /dev/null
+++ b/.gitlab/ci/review-apps/skip-qa.gitlab-ci.yml
@@ -0,0 +1,13 @@
+stages:
+  - review
+
+include:
+  - local: .gitlab/ci/global.gitlab-ci.yml
+  - local: .gitlab/ci/rules.gitlab-ci.yml
+
+no-op:
+  extends:
+    - .review:rules:start-review-app-pipeline
+  stage: review
+  script:
+    - echo "Skip Review App because the MR includes only quarantine changes"
diff --git a/.gitlab/ci/review.gitlab-ci.yml b/.gitlab/ci/review.gitlab-ci.yml
index 18bc49e77f9b..72dbb6443957 100644
--- a/.gitlab/ci/review.gitlab-ci.yml
+++ b/.gitlab/ci/review.gitlab-ci.yml
@@ -23,12 +23,42 @@ review-cleanup:
     - ruby -rrubygems scripts/review_apps/automated_cleanup.rb
     - gcp_cleanup
 
+review-app-pipeline-generate:
+  image: ${GITLAB_DEPENDENCY_PROXY}ruby:${RUBY_VERSION}
+  stage: prepare
+  extends:
+    - .review:rules:start-review-app-pipeline
+  artifacts:
+    expire_in: 7d
+    paths:
+      - ${CHANGES_DIFFS_DIR}/*
+      - review-app-pipeline.yml
+  variables:
+    CHANGES_DIFFS_DIR: tmp/diffs
+  before_script:
+    - source scripts/utils.sh
+    - install_gitlab_gem
+    - tooling/bin/find_change_diffs ${CHANGES_DIFFS_DIR}
+  script:
+    - exit_code=0 && tooling/bin/qa/run_qa_check ${CHANGES_DIFFS_DIR} || exit_code=$?
+    - |
+      if [ $exit_code -eq 0 ]; then
+        echo "Review App will use the full pipeline"
+        cp .gitlab/ci/review-apps/main.gitlab-ci.yml review-app-pipeline.yml
+      elif [ $exit_code -eq 2 ]; then
+        echo "Skip Review App because the MR includes only quarantine changes"
+        cp .gitlab/ci/review-apps/skip-qa.gitlab-ci.yml review-app-pipeline.yml
+      else
+        exit $exit_code
+      fi
+
 start-review-app-pipeline:
   extends:
     - .review:rules:start-review-app-pipeline
   resource_group: review/${CI_COMMIT_REF_SLUG}${SCHEDULE_TYPE} # CI_ENVIRONMENT_SLUG is not available here and we want this to be the same as the environment
   stage: review
   needs:
+    - review-app-pipeline-generate
     - job: build-assets-image
       artifacts: false
   # These variables are set in the pipeline schedules.
@@ -39,7 +69,8 @@ start-review-app-pipeline:
     DAST_RUN: $DAST_RUN
   trigger:
     include:
-      - local: .gitlab/ci/review-apps/main.gitlab-ci.yml
+      - artifact: review-app-pipeline.yml
+        job: review-app-pipeline-generate
     strategy: depend
 
 danger-review:
diff --git a/tooling/bin/qa/package_and_qa_check b/tooling/bin/qa/run_qa_check
similarity index 69%
rename from tooling/bin/qa/package_and_qa_check
rename to tooling/bin/qa/run_qa_check
index 21deb0fcd2dd..fa26a64f004c 100755
--- a/tooling/bin/qa/package_and_qa_check
+++ b/tooling/bin/qa/run_qa_check
@@ -3,7 +3,7 @@
 
 require 'pathname'
 
-# This script checks if the package-and-qa job should trigger downstream pipelines to run the QA suite.
+# This script checks if the code changes justify running the QA suite.
 #
 # It assumes the first argument is a directory of files containing diffs of changes from an MR
 # (e.g., created by tooling/bin/find_change_diffs). It exits with a success code if there are no diffs, or if the diffs
@@ -11,14 +11,14 @@ require 'pathname'
 #
 # The script will abort (exit code 1) if the argument is missing.
 #
-# The following condition will result in a failure code (2), indicating that package-and-qa should not run:
+# The following condition will result in a failure code (2), indicating that QA tests should not run:
 #
 #   - If the changes only include tests being put in quarantine
 
 abort("ERROR: Please specify the directory containing MR diffs.") if ARGV.empty?
 diffs_dir = Pathname.new(ARGV.shift).expand_path
 
-# Run package-and-qa if there are no diffs. E.g., in scheduled pipelines
+# Run QA tests if there are no diffs. E.g., in scheduled pipelines
 exit 0 if diffs_dir.glob('**/*').empty?
 
 files_count = 0
@@ -35,11 +35,11 @@ diffs_dir.glob('**/*').each do |path|
   quarantine_specs_count += 1 if path.read.match?(/^\+.*, quarantine:/)
 end
 
-# Run package-and-qa if there are no specs. E.g., when the MR changes QA framework files.
+# Run QA tests if there are no specs. E.g., when the MR changes QA framework files.
 exit 0 if specs_count == 0
 
-# Skip package-and-qa if there are only specs being put in quarantine.
+# Skip QA tests if there are only specs being put in quarantine.
 exit 2 if quarantine_specs_count == specs_count && quarantine_specs_count == files_count
 
-# Run package-and-qa under any other circumstances. E.g., if there are specs being put in quarantine but there are also
+# Run QA tests under any other circumstances. E.g., if there are specs being put in quarantine but there are also
 # other changes that might need to be tested.
-- 
GitLab