From 4f2db2d255d84c2c24ea857811c135dd890db1f9 Mon Sep 17 00:00:00 2001
From: Ben Henning <henning.benmax@gmail.com>
Date: Fri, 12 Feb 2021 19:31:18 -0800
Subject: [PATCH] Fix affected target computation breaking CI workflows when no
 targets are affected (#2696)

* See if the extra newline is the source of issue.

* Temporarily disable unrelated checks.

* Conditionally run tests.

While Bazel can run an empty list of tests, GitHub actions always
expects at least 1 entry for matrix-dependent actions.

This is based on
https://github.community/t/empty-matrix-fails-workflow/128736/2. It's
not yet clear if this will properly allow check_test_results to pass if
the workflow is skipped. That will need to be tested.

* Fix Bash syntax error.

* Update test results job to pass without tests run.

* Update main.yml

Reenable jobs.

* Update compute_affected_tests.sh

Undo removing the newline since it's not actually needed.

* Re-remove newline since it is needed.

I forgot that we're performing an empty check, so the extra newline does
break it.
---
 .github/workflows/unit_tests.yml  | 15 ++++++++++++---
 scripts/compute_affected_tests.sh |  2 +-
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/.github/workflows/unit_tests.yml b/.github/workflows/unit_tests.yml
index fcdd5dbe64d..d065017b830 100644
--- a/.github/workflows/unit_tests.yml
+++ b/.github/workflows/unit_tests.yml
@@ -16,6 +16,7 @@ jobs:
     runs-on: ubuntu-18.04
     outputs:
       matrix: ${{ steps.compute-test-matrix-from-affected.outputs.matrix || steps.compute-test-matrix-from-all.outputs.matrix }}
+      have_tests_to_run: ${{ steps.compute-test-matrix-from-affected.outputs.have_tests_to_run || steps.compute-test-matrix-from-all.outputs.have_tests_to_run }}
     steps:
       - uses: actions/checkout@v2
         with:
@@ -37,6 +38,11 @@ jobs:
           TEST_TARGET_LIST=$(bash ./scripts/compute_affected_tests.sh $HOME/oppia-bazel/bazel | sed 's/^\|$/"/g' | paste -sd, -)
           echo "Affected tests (note that this might be all tests if on the develop branch): $TEST_TARGET_LIST"
           echo "::set-output name=matrix::{\"test-target\":[$TEST_TARGET_LIST]}"
+          if [[ ! -z "$TEST_TARGET_LIST" ]]; then
+            echo "::set-output name=have_tests_to_run::true"
+          else
+            echo "::set-output name=have_tests_to_run::false"
+          fi
       - name: Compute test matrix based on all tests
         id: compute-test-matrix-from-all
         if: "contains(github.event.pull_request.title, '[RunAllTests]')"
@@ -44,10 +50,12 @@ jobs:
           TEST_TARGET_LIST=$($HOME/oppia-bazel/bazel query "kind(test, //...)" | sed 's/^\|$/"/g' | paste -sd, -)
           echo "Affected tests (note that this might be all tests if on the develop branch): $TEST_TARGET_LIST"
           echo "::set-output name=matrix::{\"test-target\":[$TEST_TARGET_LIST]}"
+          echo "::set-output name=have_tests_to_run::true"
 
   bazel_run_test:
     name: Run Bazel Test
     needs: bazel_compute_affected_targets
+    if: ${{ needs.bazel_compute_affected_targets.outputs.have_tests_to_run == 'true' }}
     runs-on: ubuntu-18.04
     strategy:
       fail-fast: false
@@ -110,10 +118,11 @@ jobs:
   # Reference: https://github.community/t/127354/7.
   check_test_results:
       name: Check Bazel Test Results
-      needs: bazel_run_test
+      needs: [bazel_compute_affected_targets, bazel_run_test]
       if: ${{ always() }}
       runs-on: ubuntu-18.04
       steps:
-        - name: Check tests passed
-          if: ${{ needs.bazel_run_test.result != 'success' }}
+        # This step will be skipped if there are no tests to run, so the overall job should pass.
+        - name: Check tests passed (for tests that ran)
+          if: ${{ needs.bazel_compute_affected_targets.outputs.have_tests_to_run == 'true' && needs.bazel_run_test.result != 'success' }}
           run: exit 1
diff --git a/scripts/compute_affected_tests.sh b/scripts/compute_affected_tests.sh
index e264b37d824..2181f31e190 100755
--- a/scripts/compute_affected_tests.sh
+++ b/scripts/compute_affected_tests.sh
@@ -70,7 +70,7 @@ if [[ "$current_branch" != "develop" ]]; then
   )
 
   # Print all of the affected targets without duplicates.
-  printf "%s\n" "${all_affected_targets_with_potential_duplicated[@]}" | sort -u
+  printf "%s" "${all_affected_targets_with_potential_duplicated[@]}" | sort -u
 else
   # Print all test targets.
   $BAZEL_BINARY query --noshow_progress "kind(test, //...)" 2>/dev/null