Skip to content

Commit

Permalink
Fix affected target computation breaking CI workflows when no targets…
Browse files Browse the repository at this point in the history
… 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.
  • Loading branch information
BenHenning authored Feb 13, 2021
1 parent 1657191 commit 4f2db2d
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 4 deletions.
15 changes: 12 additions & 3 deletions .github/workflows/unit_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -37,17 +38,24 @@ 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]')"
run: |
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
Expand Down Expand Up @@ -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
2 changes: 1 addition & 1 deletion scripts/compute_affected_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 4f2db2d

Please sign in to comment.