Skip to content

Commit

Permalink
Fix part of #1861: [RunAllTests] Update Bazel CI workflow to run all …
Browse files Browse the repository at this point in the history
…Oppia tests (#1904)

* Update Bazel CI workflow

Build all Oppia targets in Bazel rather than just the binary target.

* Update main.yml

Reset the workflow name so that it doesn't need to be renamed in GitHub settings.

* Introduce remote caching in Bazel.

This uses a remote storage service with a local file encrypted using
git-secret to act as the authentication key for Bazel to read & write
artifacts to the service for caching purposes.

* Add debug line.

* Disable workflows + fix debug line.

* More debugging.

* More debugging.

* Work around GitHub hiding secret since we're debugging.

* Use base64 to properly encode newlines in GPG keys.

* Remove debug lines before changing back to correct GPG key.

* Switch to production key.

* Fix env variable reference. Lock-down actions workflows via codeowners.

* Install git-secret to default location.

* Add details. Debug $PATH.

* Fix pathing for git-secret.

* Dummy commit to re-trigger actions.

* Undo commit to see if this one shows up.

* Fix git-secret pathing try 2.

* New commit to re-trigger action.

* Path debugging.

* Workaround to get GitHub to show changes.

* Update runner to use Bash.

Reference:
https://github.community/t/github-path-does-not-add-to-the-path/143992.

* Restore binary-only build, other builds, and introduce new workflow for
building all Bazel targets.

* Remove debug lines.

* Rename & remove keep_going.

* Compute matrix containing all test targets.

This will allow us to distribute parallelization responsibilities partly
to GitHub actions to hopefully have slightly better throughput. See
https://github.blog/changelog/2020-04-15-github-actions-new-workflow-features/
for reference on how this mechanism works.

* Simplify, fix some issues, and debug instead of run.

* Turn on actual testing.

* Lower parallelization since GitHub started cancelling tasks.

* Try 15 parallel jobs instead.

* Turn off fail-fast instead of limiting parallelization.

Fail fast seems to be the reason why the tests aren't completing, not
quota (since if too many jobs are started, the extras should just be
queued until resources open up).

* Simplify workflow & allow it to be required.

Also, introduce bazelrc file to simplify the CI CLIs interacting with
Bazel.

* Add test change to investigate computing affected targets.

* Another test change to compute affected targets.

* Update workflow to use future script compute_affected_tests.sh.

Also, ignore Bazel directories in Git (to ease local development).

* Add script to compute affected targets.

This script is primarily meant to be used for CI.

* Execute tests in parallel to build.

This creates a new job to compute affected targets alongside the build.
This may result in the initial build being a bit slower, but subsequent
commits should be fast if remote caching is enabled. This will also
result in better performance for forks that can't use remote caching.

* Script clean-ups.

Also, re-trigger actions.

* Try to ensure develop branch is available for change analysis.

* Add automatic workflow cancellation.

Also, add support to explicitly run all tests if '[RunAllTests]' is in
the PR title.

* Attempt to make conditional matrix computation work.

* Remove join since it was used incorrectly.

* Add support for testing when Bazel, BUILD, and WORKSPACE files change.

One consequence is the current Bazel file structure is very tight, so
any changes will likely run the whole suite. This will get better over
time.

Also, show test logs for all test runs.

* Fix broken tests by adding missing dep library.

* Finalize PR.

1. Expand codeowners to include all workflow files.
2. Remove test comments in Kotlin files.
3. Re-enable all workflows.
4. Attempt to fix tests broken on actions but not locally by adding more
thread synchronization points.

* Lint fix.

* Fix timing issues and JDK 9-specific regression.

See comment thread in #1904 for more context.

* Restore workflow names + introduce test check.

The new test check workflow will be a static job that will be required
to pass. One failing test is being introduced to verify that this check
fails as expected.

The original workflow names are being restored so that they don't need
to be renamed in GitHub settings (since that would introduce a
discontinuity in CI service & require multiple migratiaon PRs to fix).

* Update StateFragmentLocalTest.kt

Remove fail-on-purpose test since it verified what it needed to: the new test status check job is working as expected.

* Address reviewer comments.

* Fix most tests broken in Bazel after syncing.

* Gitignore + fix broken test.

The test failure here only happens when using JDK9+ (which doesn't
happen in the Gradle world, or on CI).

The .gitignore is since we can't yet specify a .bazelproject in a
shareable way.

* Lint fixes.

* Post-merge clean-up.

* Fix broken post-merge test.

* Remove unnecessary codeowners per earlier codeowners setup.

* Fix
ItemSelectionInputDoesNotContainAtLeastOneOfRuleClassifierProviderTest.

* Disable image region selection tests.

These tests can't correctly pass on Robolectric until #1611 is fixed, so
disabling them for the time being to avoid the image loading flake
happening on CI (but not locally). Note that chances are a fix will
still be needed for the flake, but that can be addressed later.

* Disable 2 previously missed tests.

* Post-merge lint fix.

* Add missing dependency.

Verified all tests build & pass both for JDK 9 & 11. Hopefully they will
work as expected on CI.

* Add missing codeowners for new files.

* Post-merge fixes.

This fixes some tests that were broken after recent PRs, and fixed a
visibility error introduced in #2663.

* Move Bazel tests to new workflow.

This will make it easier to restart failures without having to also
restart unrelated tests.
  • Loading branch information
BenHenning authored Feb 12, 2021
1 parent 8cbd6a2 commit 9173c96
Show file tree
Hide file tree
Showing 51 changed files with 572 additions and 134 deletions.
4 changes: 4 additions & 0 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ gradlew.bat @BenHenning
/.github/*.md @BenHenning
/.github/ISSUE_TEMPLATE @BenHenning

# Git secret files & related configurations.
/.gitsecret/ @BenHenning
*.secret @BenHenning

# CI configuration.
/.github/workflows/ @BenHenning

Expand Down
58 changes: 46 additions & 12 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ on:
jobs:
linters:
name: Lint Tests
runs-on: ${{ matrix.os }}
runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [ubuntu-18.04]
Expand Down Expand Up @@ -65,7 +65,7 @@ jobs:

robolectric_tests:
name: Robolectric Tests (Non-App Modules)
runs-on: ${{ matrix.os }}
runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [ubuntu-18.04]
Expand Down Expand Up @@ -122,7 +122,7 @@ jobs:

app_tests:
name: Robolectric Tests - App Module (Non-Flaky)
runs-on: ${{ matrix.os }}
runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [ubuntu-18.04]
Expand Down Expand Up @@ -158,32 +158,66 @@ jobs:
os: [ubuntu-18.04]
steps:
- uses: actions/checkout@v2
- name: Set up bazel
uses: jwlawson/actions-setup-bazel@v1
with:
bazel-version: '3.4.1'
- name: Clone Oppia Bazel
run: git clone https://github.com/oppia/bazel.git $HOME/oppia-bazel
- name: Set up JDK 9
uses: actions/setup-java@v1
with:
java-version: 9
- name: Extract Android Tools
- name: Extract Android tools
run: |
mkdir -p $GITHUB_WORKSPACE/tmp/android_tools
cd $HOME/oppia-bazel
unzip bazel-tools.zip
tar -xf $HOME/oppia-bazel/android_tools.tar.gz -C $GITHUB_WORKSPACE/tmp/android_tools
- name: Unzip Bazel Binary and Build Oppia
# See https://git-secret.io/installation for details on installing git-secret. Note that the
# apt-get method isn't used since it's much slower to update & upgrade apt before installation
# versus just directly cloning & installing the project. Further, the specific version
# shouldn't matter since git-secret relies on a future-proof storage mechanism for secrets.
# This also uses a different directory to install git-secret to avoid requiring root access
# when running the git secret command.
- name: Install git-secret (non-fork only)
if: github.repository == 'oppia/oppia-android'
shell: bash
run: |
cd $HOME
mkdir -p $HOME/gitsecret
git clone https://github.com/sobolevn/git-secret.git git-secret
cd git-secret && make build
PREFIX="$HOME/gitsecret" make install
echo "$HOME/gitsecret" >> $GITHUB_PATH
echo "$HOME/gitsecret/bin" >> $GITHUB_PATH
- name: Decrypt secrets (non-fork only)
if: github.repository == 'oppia/oppia-android'
env:
GIT_SECRET_GPG_PRIVATE_KEY: ${{ secrets.GIT_SECRET_GPG_PRIVATE_KEY }}
run: |
cd $HOME
# NOTE TO DEVELOPERS: Make sure to never print this key directly to stdout!
echo $GIT_SECRET_GPG_PRIVATE_KEY | base64 --decode > ./git_secret_private_key.gpg
gpg --import ./git_secret_private_key.gpg
cd $GITHUB_WORKSPACE
git secret reveal
- name: Unzip Bazel binary
run: |
cd $HOME/oppia-bazel
unzip bazel-build.zip
cd $GITHUB_WORKSPACE
chmod a+x $HOME/oppia-bazel/bazel
$HOME/oppia-bazel/bazel build //:oppia --android_databinding_use_v3_4_args --experimental_android_databinding_v2 --override_repository=android_tools=$GITHUB_WORKSPACE/tmp/android_tools --java_header_compilation=false --noincremental_dexing --define=android_standalone_dexing_tool=d8_compat_dx
cp $GITHUB_WORKSPACE/bazel-bin/oppia.apk /home/runner/work/oppia-android/oppia-android/
# Note that caching only works on non-forks.
- name: Build Oppia binary (with caching, non-fork only)
if: github.repository == 'oppia/oppia-android'
env:
BAZEL_REMOTE_CACHE_URL: ${{ secrets.BAZEL_REMOTE_CACHE_URL }}
run: |
$HOME/oppia-bazel/bazel build --override_repository=android_tools=$GITHUB_WORKSPACE/tmp/android_tools --remote_http_cache=$BAZEL_REMOTE_CACHE_URL --google_credentials=./config/oppia-dev-workflow-remote-cache-credentials.json -- //:oppia
- name: Build Oppia binary (without caching, fork only)
if: github.repository != 'oppia/oppia-android'
run: |
$HOME/oppia-bazel/bazel build --override_repository=android_tools=$GITHUB_WORKSPACE/tmp/android_tools -- //:oppia
- name: Copy Oppia APK for uploading
run: cp $GITHUB_WORKSPACE/bazel-bin/oppia.apk /home/runner/work/oppia-android/oppia-android/
- uses: actions/upload-artifact@v2
with:
name: oppia-bazel-apk
path: /home/runner/work/oppia-android/oppia-android/oppia.apk
- uses: actions/checkout@v2
118 changes: 118 additions & 0 deletions .github/workflows/unit_tests.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
name: Unit Tests (Robolectric - Bazel)

# Controls when the action will run. Triggers the workflow on pull request
# events or push events in the develop branch.
on:
workflow_dispatch:
pull_request:
push:
branches:
# Push events on develop branch
- develop

jobs:
bazel_compute_affected_targets:
name: Compute affected tests
runs-on: ubuntu-18.04
outputs:
matrix: ${{ steps.compute-test-matrix-from-affected.outputs.matrix || steps.compute-test-matrix-from-all.outputs.matrix }}
steps:
- uses: actions/checkout@v2
with:
fetch-depth: 0
- name: Clone Oppia Bazel
run: git clone https://github.com/oppia/bazel.git $HOME/oppia-bazel
- name: Unzip Bazel binary
run: |
cd $HOME/oppia-bazel
unzip bazel-build.zip
cd $GITHUB_WORKSPACE
chmod a+x $HOME/oppia-bazel/bazel
- name: Compute test matrix based on affected targets
id: compute-test-matrix-from-affected
if: "!contains(github.event.pull_request.title, '[RunAllTests]')"
# https://unix.stackexchange.com/a/338124 for reference on creating a JSON-friendly
# comma-separated list of test targets for the matrix.
run: |
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]}"
- 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]}"
bazel_run_test:
name: Run Bazel Test
needs: bazel_compute_affected_targets
runs-on: ubuntu-18.04
strategy:
fail-fast: false
matrix: ${{fromJson(needs.bazel_compute_affected_targets.outputs.matrix)}}
steps:
- uses: actions/checkout@v2
- name: Clone Oppia Bazel
run: git clone https://github.com/oppia/bazel.git $HOME/oppia-bazel
- name: Set up JDK 9
uses: actions/setup-java@v1
with:
java-version: 9
- name: Extract Android tools
run: |
mkdir -p $GITHUB_WORKSPACE/tmp/android_tools
cd $HOME/oppia-bazel
unzip bazel-tools.zip
tar -xf $HOME/oppia-bazel/android_tools.tar.gz -C $GITHUB_WORKSPACE/tmp/android_tools
# See explanation in bazel_build_app for how this is installed.
- name: Install git-secret (non-fork only)
if: github.repository == 'oppia/oppia-android'
shell: bash
run: |
cd $HOME
mkdir -p $HOME/gitsecret
git clone https://github.com/sobolevn/git-secret.git git-secret
cd git-secret && make build
PREFIX="$HOME/gitsecret" make install
echo "$HOME/gitsecret" >> $GITHUB_PATH
echo "$HOME/gitsecret/bin" >> $GITHUB_PATH
- name: Decrypt secrets (non-fork only)
if: github.repository == 'oppia/oppia-android'
env:
GIT_SECRET_GPG_PRIVATE_KEY: ${{ secrets.GIT_SECRET_GPG_PRIVATE_KEY }}
run: |
cd $HOME
# NOTE TO DEVELOPERS: Make sure to never print this key directly to stdout!
echo $GIT_SECRET_GPG_PRIVATE_KEY | base64 --decode > ./git_secret_private_key.gpg
gpg --import ./git_secret_private_key.gpg
cd $GITHUB_WORKSPACE
git secret reveal
- name: Unzip Bazel binary
run: |
cd $HOME/oppia-bazel
unzip bazel-build.zip
cd $GITHUB_WORKSPACE
chmod a+x $HOME/oppia-bazel/bazel
- name: Run Oppia Test (with caching, non-fork only)
if: github.repository == 'oppia/oppia-android'
env:
BAZEL_REMOTE_CACHE_URL: ${{ secrets.BAZEL_REMOTE_CACHE_URL }}
run: $HOME/oppia-bazel/bazel test --override_repository=android_tools=$GITHUB_WORKSPACE/tmp/android_tools --remote_http_cache=$BAZEL_REMOTE_CACHE_URL --google_credentials=./config/oppia-dev-workflow-remote-cache-credentials.json -- ${{ matrix.test-target }}
- name: Run Oppia Test (without caching, fork only)
if: github.repository != 'oppia/oppia-android'
env:
BAZEL_REMOTE_CACHE_URL: ${{ secrets.BAZEL_REMOTE_CACHE_URL }}
run: $HOME/oppia-bazel/bazel test --override_repository=android_tools=$GITHUB_WORKSPACE/tmp/android_tools -- ${{ matrix.test-target }}

# Reference: https://github.community/t/127354/7.
check_test_results:
name: Check Bazel Test Results
needs: bazel_run_test
if: ${{ always() }}
runs-on: ubuntu-18.04
steps:
- name: Check tests passed
if: ${{ needs.bazel_run_test.result != 'success' }}
run: exit 1
28 changes: 28 additions & 0 deletions .github/workflows/workflow_canceller.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
name: Automatic Workflow Canceller

# This workflow should be triggered in one of three situations:
# 1. Manual workflow dispatch via https://github.com/oppia/oppia-android/actions.
# 2. Upon creation of a PR & updates to that PR.
# 3. Whenever the develop branch is changed (e.g. after a PR is merged).
#
# Note that the action being used here automatically accounts for the current branch & the commit
# hash of the tip of the branch to ensure it doesn't cancel previous workflows that aren't related
# to the branch being evaluated.
on:
workflow_dispatch:
pull_request:
push:
branches:
# Push events on develop branch
- develop

jobs:
cancel:
name: Cancel Previous Runs
runs-on: ubuntu-18.04
steps:
# See https://github.com/styfle/cancel-workflow-action for details on this workflow.
- uses: styfle/[email protected]
with:
workflow_id: main.yml
access_token: ${{ github.token }}
5 changes: 5 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,8 @@ utility/build
gradle
gradlew
gradlew.bat
.gitsecret/keys/random_seed
!*.secret
config/oppia-dev-workflow-remote-cache-credentials.json
bazel-*
.bazelproject
Binary file added .gitsecret/keys/pubring.kbx
Binary file not shown.
Binary file added .gitsecret/keys/pubring.kbx~
Binary file not shown.
Binary file added .gitsecret/keys/trustdb.gpg
Binary file not shown.
1 change: 1 addition & 0 deletions .gitsecret/paths/mapping.cfg
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
config/oppia-dev-workflow-remote-cache-credentials.json:c02f95d3829f9a7fb3757170ade432efa43d562d2e7c208372ec9d0f4a45da1a
1 change: 1 addition & 0 deletions app/src/main/res/layout-land/submitted_answer_item.xml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
</data>

<androidx.constraintlayout.widget.ConstraintLayout
android:id="@+id/submitted_answer_container"
android:layout_width="match_parent"
android:layout_height="wrap_content"
app:explorationSplitViewMarginApplicable="@{viewModel.hasConversationView &amp;&amp; viewModel.isSplitView}"
Expand Down
1 change: 1 addition & 0 deletions app/src/main/res/layout/submitted_answer_item.xml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
</data>

<androidx.constraintlayout.widget.ConstraintLayout
android:id="@+id/submitted_answer_container"
android:layout_width="match_parent"
android:layout_height="wrap_content"
app:explorationSplitViewMarginApplicable="@{viewModel.hasConversationView &amp;&amp; viewModel.isSplitView}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,7 @@ class StateFragmentTest {
}

@Test
@RunOn(TestPlatform.ESPRESSO) // TODO(#1611): Enable for Robolectric.
fun testStateFragment_loadImageRegion_clickRegion6_submitButtonClickable() {
launchForExploration(TEST_EXPLORATION_ID_5).use {
startPlayingExploration()
Expand All @@ -564,6 +565,7 @@ class StateFragmentTest {
}

@Test
@RunOn(TestPlatform.ESPRESSO) // TODO(#1611): Enable for Robolectric.
fun testStateFragment_loadImageRegion_clickRegion6_clickSubmit_receivesCorrectFeedback() {
launchForExploration(TEST_EXPLORATION_ID_5).use {
startPlayingExploration()
Expand All @@ -582,6 +584,7 @@ class StateFragmentTest {
}

@Test
@RunOn(TestPlatform.ESPRESSO) // TODO(#1611): Enable for Robolectric.
fun testStateFragment_loadImageRegion_submitButtonDisabled() {
launchForExploration(TEST_EXPLORATION_ID_5).use {
startPlayingExploration()
Expand All @@ -595,7 +598,7 @@ class StateFragmentTest {

@Test
@RunOn(TestPlatform.ESPRESSO) // TODO(#1611): Enable for Robolectric.
fun loadImageRegion_defaultRegionClick_defaultRegionClicked_submitButtonDisabled() {
fun testStateFragment_loadImageRegion_defaultRegionClick_defRegionClicked_submitButtonDisabled() {
launchForExploration(TEST_EXPLORATION_ID_5).use {
startPlayingExploration()
waitForImageViewInteractionToFullyLoad()
Expand All @@ -608,6 +611,7 @@ class StateFragmentTest {
}

@Test
@RunOn(TestPlatform.ESPRESSO) // TODO(#1611): Enable for Robolectric.
fun testStateFragment_loadImageRegion_clickedRegion6_region6Clicked_submitButtonEnabled() {
launchForExploration(TEST_EXPLORATION_ID_5).use {
startPlayingExploration()
Expand All @@ -621,6 +625,7 @@ class StateFragmentTest {
}

@Test
@RunOn(TestPlatform.ESPRESSO) // TODO(#1611): Enable for Robolectric.
fun testStateFragment_loadImageRegion_clickedRegion6_region6Clicked_correctFeedback() {
launchForExploration(TEST_EXPLORATION_ID_5).use {
startPlayingExploration()
Expand All @@ -639,6 +644,7 @@ class StateFragmentTest {
}

@Test
@RunOn(TestPlatform.ESPRESSO) // TODO(#1611): Enable for Robolectric.
fun testStateFragment_loadImageRegion_clickedRegion6_region6Clicked_correctAnswer() {
launchForExploration(TEST_EXPLORATION_ID_5).use {
startPlayingExploration()
Expand All @@ -657,6 +663,7 @@ class StateFragmentTest {
}

@Test
@RunOn(TestPlatform.ESPRESSO) // TODO(#1611): Enable for Robolectric.
fun testStateFragment_loadImageRegion_clickedRegion6_region6Clicked_continueButtonIsDisplayed() {
launchForExploration(TEST_EXPLORATION_ID_5).use {
startPlayingExploration()
Expand All @@ -671,7 +678,8 @@ class StateFragmentTest {
}

@Test
fun loadImageRegion_clickRegion6_clickedRegion5_region5Clicked_correctFeedback() {
@RunOn(TestPlatform.ESPRESSO) // TODO(#1611): Enable for Robolectric.
fun testStateFragment_loadImageRegion_clickRegion6_clickedRegion5_clickRegion5_correctFeedback() {
launchForExploration(TEST_EXPLORATION_ID_5).use {
startPlayingExploration()
waitForImageViewInteractionToFullyLoad()
Expand Down
Loading

0 comments on commit 9173c96

Please sign in to comment.