Skip to content

Commit

Permalink
Fix oppia#2711 & part of oppia#5343: Workflow cancellation support in…
Browse files Browse the repository at this point in the history
… CI using concurrency (oppia#5466)

<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->

Fix oppia#2711 
Fixes part of oppia#5343 

### Project
[PR 2.5 of Project 4.1]

### Changes Made
- The existing workflow canceller utilized the
[cancel-workflow-action](https://github.com/styfle/cancel-workflow-action)
by syfle.
- Since GitHub introduced native
[concurrency](https://docs.github.com/en/actions/using-jobs/using-concurrency)
support on April 19, 2021, we can now utilize this built-in feature for
better integration.
- This implementation leverages the [concurrency
property](https://docs.github.com/en/actions/using-jobs/using-concurrency#example-using-concurrency-and-the-default-behavior)
to manage workflows, using the configuration:
```
concurrency:
  group: ${{ github.workflow }}-${{ github.ref }}
  cancel-in-progress: true
```
- This setup groups workflows by their name and reference, allowing
GitHub to automatically cancel any in-progress jobs in the workflow when
a new workflow run is triggered.
- This retires the existing Automatic workflow canceller
(workflow_canceller.yml).
- This approach is now in place for the following workflows: 
  - Build Tests 
  - Static Checks 
  - Unit Tests (Robolectric - Gradle)
  - Unit Tests (Robolectric - Bazel)

### Triggered Cancellation
(PR Check Triggers) ->
https://github.com/oppia/oppia-android/pull/5466/checks

### Screenshots of workflow cancelled
**Workflows Running:**

![Screenshot
(1536)](https://github.com/user-attachments/assets/fa11a887-0bee-4d6c-8c58-fa2e723bde91)

**Workflows in previous run cancelling after latest commit**

![Screenshot
(1538)](https://github.com/user-attachments/assets/14d96ff7-38c6-4054-a084-7712c34b9c29)

**Note:** It is observed that the Non-App Module runs take a bit of time
to cancel but they eventually get cancelled.


![image](https://github.com/user-attachments/assets/516628ee-15a0-4a95-926c-ea925357bae2)


![image](https://github.com/user-attachments/assets/03a74685-c458-44a8-ada9-09faf45eeb93)

## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [x] The PR title and explanation each start with "Fix #bugnum: " (If
this PR fixes part of an issue, prefix the title with "Fix part of
#bugnum: ...".)
- [x] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).
  • Loading branch information
Rd4dev authored Jul 29, 2024
1 parent d2db9df commit f892744
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 44 deletions.
4 changes: 4 additions & 0 deletions .github/workflows/build_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ on:
# Push events on develop branch
- develop

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

jobs:
bazel_build_app:
name: Build Binary with Bazel
Expand Down
34 changes: 24 additions & 10 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ on:
# Push events on develop branch
- develop

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

# This workflow has the following jobs:
# robolectric_tests: Robolectric tests for all modules except the app module
# app_tests: Non-flaky Robolectric tests for the app module
Expand Down Expand Up @@ -54,45 +58,53 @@ jobs:
run: sudo ./gradlew --full-stacktrace assembleDebug -Dorg.gradle.java.home=$JAVA_HOME

- name: Utility tests
if: always()
# The expression if: ${{ !cancelled() }} runs a job or step regardless of its success or failure while responding to cancellations,
# serving as a cancellation-compliant alternative to if: ${{ always() }} in concurrent workflows.
if: ${{ !cancelled() }}
# We require 'sudo' to avoid an error of the existing android sdk. See https://github.com/actions/starter-workflows/issues/58
run: sudo ./gradlew --full-stacktrace :utility:testDebugUnitTest -Dorg.gradle.java.home=$JAVA_HOME
- name: Upload Utility Test Reports
uses: actions/upload-artifact@v2
if: ${{ always() }} # IMPORTANT: Upload reports regardless of status
if: ${{ !cancelled() }} # IMPORTANT: Upload reports regardless of success or failure status
with:
name: utility reports
path: utility/build/reports

- name: Data tests
if: always()
# The expression if: ${{ !cancelled() }} runs a job or step regardless of its success or failure while responding to cancellations,
# serving as a cancellation-compliant alternative to if: ${{ always() }} in concurrent workflows.
if: ${{ !cancelled() }}
# We require 'sudo' to avoid an error of the existing android sdk. See https://github.com/actions/starter-workflows/issues/58
run: sudo ./gradlew --full-stacktrace :data:testDebugUnitTest -Dorg.gradle.java.home=$JAVA_HOME
- name: Upload Data Test Reports
uses: actions/upload-artifact@v2
if: ${{ always() }} # IMPORTANT: Upload reports regardless of status
if: ${{ !cancelled() }} # IMPORTANT: Upload reports regardless of success or failure status
with:
name: data reports
path: data/build/reports

- name: Domain tests
if: always()
# The expression if: ${{ !cancelled() }} runs a job or step regardless of its success or failure while responding to cancellations,
# serving as a cancellation-compliant alternative to if: ${{ always() }} in concurrent workflows.
if: ${{ !cancelled() }}
# We require 'sudo' to avoid an error of the existing android sdk. See https://github.com/actions/starter-workflows/issues/58
run: sudo ./gradlew --full-stacktrace :domain:testDebugUnitTest -Dorg.gradle.java.home=$JAVA_HOME
- name: Upload Domain Test Reports
uses: actions/upload-artifact@v2
if: ${{ always() }} # IMPORTANT: Upload reports regardless of status
if: ${{ !cancelled() }} # IMPORTANT: Upload reports regardless of success or failure status
with:
name: domain reports
path: domain/build/reports

- name: Testing tests
if: always()
# The expression if: ${{ !cancelled() }} runs a job or step regardless of its success or failure while responding to cancellations,
# serving as a cancellation-compliant alternative to if: ${{ always() }} in concurrent workflows.
if: ${{ !cancelled() }}
# We require 'sudo' to avoid an error of the existing android sdk. See https://github.com/actions/starter-workflows/issues/58
run: sudo ./gradlew --full-stacktrace :testing:testDebugUnitTest -Dorg.gradle.java.home=$JAVA_HOME
- name: Upload Testing Test Reports
uses: actions/upload-artifact@v2
if: ${{ always() }} # IMPORTANT: Upload reports regardless of status
if: ${{ !cancelled() }} # IMPORTANT: Upload reports regardless of success or failure status
with:
name: testing reports
path: testing/build/reports
Expand Down Expand Up @@ -133,15 +145,17 @@ jobs:
sudo ./gradlew --full-stacktrace :app:testDebugUnitTest --${{ matrix.shard }} -Dorg.gradle.java.home=$JAVA_HOME
- name: Upload App Test Reports
uses: actions/upload-artifact@v2
if: ${{ always() }} # IMPORTANT: Upload reports regardless of status
if: ${{ !cancelled() }} # IMPORTANT: Upload reports regardless of success or failure status
with:
name: app reports ${{ matrix.shard }}
path: app/build/reports

app_tests:
name: App Module Robolectric Tests
needs: run_app_module_test
if: ${{ always() }}
# The expression if: ${{ !cancelled() }} runs a job or step regardless of its success or failure while responding to cancellations,
# serving as a cancellation-compliant alternative to if: ${{ always() }} in concurrent workflows.
if: ${{ !cancelled() }}
runs-on: ${{ matrix.os }}
strategy:
matrix:
Expand Down
44 changes: 34 additions & 10 deletions .github/workflows/static_checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ on:
branches:
- develop

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

jobs:
check_codeowners:
name: Check CODEOWNERS & Repository files
Expand Down Expand Up @@ -151,39 +155,53 @@ jobs:
shell: bash

- name: Regex Patterns Validation Check
if: always()
# The expression if: ${{ !cancelled() }} runs a job or step regardless of its success or failure while responding to cancellations,
# serving as a cancellation-compliant alternative to if: ${{ always() }} in concurrent workflows.
if: ${{ !cancelled() }}
run: |
bazel run //scripts:regex_pattern_validation_check -- $(pwd)
- name: XML Syntax Validation Check
if: always()
# The expression if: ${{ !cancelled() }} runs a job or step regardless of its success or failure while responding to cancellations,
# serving as a cancellation-compliant alternative to if: ${{ always() }} in concurrent workflows.
if: ${{ !cancelled() }}
run: |
bazel run //scripts:xml_syntax_check -- $(pwd)
- name: Testfile Presence Check
if: always()
# The expression if: ${{ !cancelled() }} runs a job or step regardless of its success or failure while responding to cancellations,
# serving as a cancellation-compliant alternative to if: ${{ always() }} in concurrent workflows.
if: ${{ !cancelled() }}
run: |
bazel run //scripts:test_file_check -- $(pwd)
- name: Accessibility label Check
if: always()
# The expression if: ${{ !cancelled() }} runs a job or step regardless of its success or failure while responding to cancellations,
# serving as a cancellation-compliant alternative to if: ${{ always() }} in concurrent workflows.
if: ${{ !cancelled() }}
run: |
bazel run //scripts:accessibility_label_check -- $(pwd) scripts/assets/accessibility_label_exemptions.pb app/src/main/AndroidManifest.xml
- name: KDoc Validation Check
if: always()
# The expression if: ${{ !cancelled() }} runs a job or step regardless of its success or failure while responding to cancellations,
# serving as a cancellation-compliant alternative to if: ${{ always() }} in concurrent workflows.
if: ${{ !cancelled() }}
run: |
bazel run //scripts:kdoc_validity_check -- $(pwd) scripts/assets/kdoc_validity_exemptions.pb
- name: Todo Check
if: always()
# The expression if: ${{ !cancelled() }} runs a job or step regardless of its success or failure while responding to cancellations,
# serving as a cancellation-compliant alternative to if: ${{ always() }} in concurrent workflows.
if: ${{ !cancelled() }}
env:
GITHUB_TOKEN: ${{ github.token }}
run: |
bazel run //scripts:todo_open_check -- $(pwd) scripts/assets/todo_open_exemptions.pb
- name: String Resource Validation Check
if: always()
# The expression if: ${{ !cancelled() }} runs a job or step regardless of its success or failure while responding to cancellations,
# serving as a cancellation-compliant alternative to if: ${{ always() }} in concurrent workflows.
if: ${{ !cancelled() }}
run: |
bazel run //scripts:string_resource_validation_check -- $(pwd)
Expand All @@ -202,16 +220,22 @@ jobs:
version: 6.5.0

- name: Maven Repin Check
if: always()
# The expression if: ${{ !cancelled() }} runs a job or step regardless of its success or failure while responding to cancellations,
# serving as a cancellation-compliant alternative to if: ${{ always() }} in concurrent workflows.
if: ${{ !cancelled() }}
run: |
REPIN=1 bazel run @unpinned_maven//:pin
- name: Maven Dependencies Update Check
if: always()
# The expression if: ${{ !cancelled() }} runs a job or step regardless of its success or failure while responding to cancellations,
# serving as a cancellation-compliant alternative to if: ${{ always() }} in concurrent workflows.
if: ${{ !cancelled() }}
run: |
bazel run //scripts:maven_dependencies_list_check -- $(pwd) third_party/maven_install.json scripts/assets/maven_dependencies.pb
- name: License Texts Check
if: always()
# The expression if: ${{ !cancelled() }} runs a job or step regardless of its success or failure while responding to cancellations,
# serving as a cancellation-compliant alternative to if: ${{ always() }} in concurrent workflows.
if: ${{ !cancelled() }}
run: |
bazel run //scripts:license_texts_check -- $(pwd)/app/src/main/res/values/third_party_dependencies.xml
8 changes: 7 additions & 1 deletion .github/workflows/unit_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ on:
# Push events on develop branch
- develop

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

jobs:
bazel_compute_affected_targets:
name: Compute affected tests
Expand Down Expand Up @@ -320,7 +324,9 @@ jobs:
check_test_results:
name: Check Bazel Test Results
needs: [bazel_compute_affected_targets, bazel_run_test]
if: ${{ always() }}
# The expression if: ${{ !cancelled() }} runs a job or step regardless of its success or failure while responding to cancellations,
# serving as a cancellation-compliant alternative to if: ${{ always() }} in concurrent workflows.
if: ${{ !cancelled() }}
runs-on: ubuntu-20.04
steps:
# This step will be skipped if there are no tests to run, so the overall job should pass.
Expand Down
23 changes: 0 additions & 23 deletions .github/workflows/workflow_canceller.yml

This file was deleted.

0 comments on commit f892744

Please sign in to comment.