Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .github/actions/setup-java/action.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
name: 'Setup Java & Gradle'
description: 'Sets up JDK 17 with Gradle caching'
runs:
using: "composite"
steps:
- name: Set up JDK
uses: actions/setup-java@v5
with:
java-version: '17'
distribution: 'adopt'
cache: gradle
139 changes: 105 additions & 34 deletions .github/workflows/_build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,44 @@ env:

jobs:

Build:
name: Build
# ============================================
# PARALLEL JOB GROUP 1: Static Analysis & Tests
# These jobs run independently and in parallel
# ============================================

StaticAnalysis:
name: DetektAndLint
runs-on: ubuntu-24.04-16cores-public
timeout-minutes: 30
timeout-minutes: 15

steps:
- uses: actions/checkout@v6

- uses: ./.github/actions/setup-java

- name: Detekt check
run: ./gradlew detekt -PdisablePreDex

- name: Lint check
run: ./gradlew lint${{ env.flavour }}${{ env.config }} -PdisablePreDex

UnitTests:
name: Unit Tests
runs-on: ubuntu-24.04-16cores-public
timeout-minutes: 15

steps:
- uses: actions/checkout@v6

- uses: ./.github/actions/setup-java

- name: Unit Tests
run: ./gradlew test${{ env.flavour }}${{ env.config }}UnitTest -PdisablePreDex

BuildAndTokens:
name: Build & Tokens
runs-on: ubuntu-24.04-16cores-public
timeout-minutes: 20
permissions:
statuses: write
pull-requests: write
Expand Down Expand Up @@ -58,12 +92,7 @@ jobs:
env:
GITHUB_TOKEN: ${{ steps.app-token.outputs.token }}

- name: Set up JDK
uses: actions/setup-java@v5
with:
java-version: '17'
distribution: 'adopt'
cache: gradle
- uses: ./.github/actions/setup-java

- name: Validate Gradle Wrapper
uses: gradle/actions/wrapper-validation@v5
Expand Down Expand Up @@ -96,14 +125,10 @@ jobs:
git diff-index --quiet HEAD || git commit -m "Update tokens"
git push

- name: Detekt check
run: ./gradlew detekt -PdisablePreDex

- name: Lint check
run: ./gradlew lint${{ env.flavour }}${{ env.config }} -PdisablePreDex

- name: Unit Tests
run: ./gradlew test${{ env.flavour }}${{ env.config }}UnitTest -PdisablePreDex
# ============================================
# PARALLEL JOB GROUP 2: Instrumented Tests
# These jobs also run in parallel with Group 1
# ============================================

Android:
name: Android tests
Expand All @@ -121,12 +146,7 @@ jobs:
sudo udevadm control --reload-rules
sudo udevadm trigger --name-match=kvm

- name: Set up JDK
uses: actions/setup-java@v5
with:
java-version: '17'
distribution: 'adopt'
cache: gradle
- uses: ./.github/actions/setup-java

- name: AVD cache
uses: actions/cache@v4
Expand Down Expand Up @@ -169,13 +189,54 @@ jobs:

./gradlew :Backpack:connected${{ env.config }}AndroidTest :backpack-compose:connected${{ env.config }}AndroidTest :backpack-common:connected${{ env.config }}AndroidTest && killall -INT crashpad_handler || true

# ============================================
# Screenshot Tests - Parallelized by Variant
# Each variant runs independently to reduce wall-clock time
# Using matrix strategy to eliminate duplication
# ============================================

Screenshots:
name: Screenshots tests
name: Screenshots (${{ matrix.variant.name }})
runs-on: ubuntu-24.04-16cores-public
timeout-minutes: 10
strategy:
matrix:
variant:
- name: default
flag: default
- name: dark mode
flag: dm
- name: RTL
flag: rtl
- name: themed
flag: themed

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a blocker
Is there a way to make this dynamic, instead of setting all variants manually could we read existing variants in the repo and extract the variant names

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is possible to extract variant from BpkTestVariant. However:
Trade-offs:

  • Pro: Single source of truth (the Kotlin enum)
  • Pro: Automatically picks up new variants
  • Con: Adds complexity and another job dependency
  • Con: Harder to debug when something goes wrong
  • Con: The merge step (line 290) would also need to be updated to be dynamic
  • Con: Less explicit - harder to see what variants run at a glance

My recommendation:
Keep it hardcoded for now. The variants rarely change (4 variants in the entire project history), and the explicit list makes the workflow easier to understand and debug. The maintenance burden of keeping two lists in
sync is minimal compared to the complexity of parsing Kotlin in bash.

steps:
- name: Checkout
uses: actions/checkout@v6

- uses: ./.github/actions/setup-java

- name: Screenshot Tests - ${{ matrix.variant.name }}
run: |
set -e
rm -rf app/screenshots/oss
./gradlew app:recordRoborazziOssDebug -Dvariant=${{ matrix.variant.flag }}

- name: Upload Screenshots
uses: actions/upload-artifact@v4
with:
name: screenshots-${{ matrix.variant.flag }}
path: app/screenshots/
retention-days: 1

Screenshots-Collect:
name: Collect & commit screenshots
runs-on: ubuntu-24.04-16cores-public
needs: Screenshots
permissions:
pull-requests: write
contents: write
timeout-minutes: 20
timeout-minutes: 5
Copy link
Contributor

@LokmaneKrizou Lokmane Krizou (LokmaneKrizou) Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we confident about dropping the timeout to 5 minutes? That’s a pretty aggressive reduction (20 → 5). If we’re confident that, after splitting, each job will consistently finish within 5 minutes, then I’m happy to keep it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why 5 minutes should be sufficient:

  • This job doesn't run any tests or builds - it only downloads and merges artifacts
  • Artifact downloads are typically fast on GitHub Actions
  • File operations are simple bash commands (mkdir, cp, rm)
  • The previous 20-minute timeout was likely inherited from when this job also ran the actual screenshot tests

Why it was safe to reduce:

  • The actual screenshot tests now run in parallel in the separate Screenshots matrix job (which has 10-minute timeout)
  • The collect job only does I/O operations, which are much faster than running tests
  • Even with network delays, 5 minutes provides a ~3-4x safety margin for this type of work

steps:
- uses: actions/create-github-app-token@v2
id: app-token
Expand All @@ -188,18 +249,28 @@ jobs:
with:
token: ${{ steps.app-token.outputs.token }}

- name: Set up JDK
uses: actions/setup-java@v5
- name: Download all screenshot artifacts
uses: actions/download-artifact@v4
with:
java-version: '17'
distribution: 'adopt'
cache: gradle
path: screenshot-artifacts

- name: Screenshot Tests
id: screenshotTests
- name: Combine screenshot results
id: combineScreenshots
run: |
set -e
./scripts/record_screenshot_tests.sh
rm -rf app/screenshots/oss
mkdir -p app/screenshots/oss

# Copy all screenshots from artifacts with error checking
for variant in default dm rtl themed; do
artifact_dir="screenshot-artifacts/screenshots-$variant/oss"
if [ -d "$artifact_dir" ] && compgen -G "$artifact_dir/*" > /dev/null; then
echo "Copying screenshots for variant: $variant"
cp -r "$artifact_dir"/* app/screenshots/oss/
else
echo "Warning: No screenshots found for variant: $variant"
fi
done

changedFiles=`git status --porcelain` && echo "CHANGED_FILES=${changedFiles//$'\n'/'%0A'}" >> $GITHUB_OUTPUT

Expand All @@ -209,7 +280,7 @@ jobs:
run: ./scripts/check-no-changes.sh

- name: Commit changes
if: ${{ github.event_name == 'pull_request' && steps.screenshotTests.outputs.CHANGED_FILES != '' }}
if: ${{ github.event_name == 'pull_request' && steps.combineScreenshots.outputs.CHANGED_FILES != '' }}
run: |
git config --local user.email "197108191+skyscanner-backpack-bot[bot]@users.noreply.github.com"
git config --local user.name "skyscanner-backpack-bot[bot]"
Expand Down