Skip to content

CATROID-1655 Add update smoke CI workflow - #5205

Open
wslany wants to merge 2 commits into
Catrobat:developfrom
wslany:codex/add-update-smoke-ci
Open

CATROID-1655 Add update smoke CI workflow#5205
wslany wants to merge 2 commits into
Catrobat:developfrom
wslany:codex/add-update-smoke-ci

Conversation

@wslany

@wslany wslany commented Apr 25, 2026

Copy link
Copy Markdown
Member

This PR adds a GitHub Actions update smoke test to catch regressions that only appear when users update an existing Catroid installation.

https://catrobat.atlassian.net/browse/CATROID-1655

What this tests

This workflow simulates the real Play Store update path:

  1. Build or restore a release-style baseline APK from the currently deployed Play Store branch (main).
  2. Build a release-like updateTest APK for the PR with the same package name and baselineVersionCode + 1.
  3. Install the baseline APK without launching it.
  4. Seed private app storage directly with legacy/update fixtures.
  5. Update in place to the PR APK.
  6. Launch the updated app and perform a small UI smoke flow.

Both APKs are built locally and signed with the same debug key. This is intentional for CI; the test does not update over an actually Play-signed APK.

CI Cost And Caching

The baseline APK cache is keyed by the resolved Play Store baseline commit SHA plus Gradle/build-file hash. As long as the deployed baseline branch does not move, later PRs can reuse the baseline APK instead of rebuilding it.

The workflow ignores docs/Markdown-only changes, uses a concurrency group to cancel stale runs, and has a 35-minute timeout.

Fixtures

The test seeds:

  • Pong Starter as a known-good project.
  • UpdateSmokeBrokenProject with malformed code.xml and a corrupt thumbnail.
  • automationScripts/update-smoke/fixtures/legacy-room-v2-app-database.sql, a Room v2 database fixture with user_version=2, identity hash c60ebf..., and a project_response table missing the v3 private column.

Diagnostics

On failure, the script captures logcat and a UIAutomator window dump. CI uploads these artifacts via actions/upload-artifact.

Verification result:

  • Fixed branch CATROID-1654 Fix startup crash by hardening project parsing and room upgrade #5203 + smoke test: passed end-to-end.
    It launched, accepted first-run dialog, opened Pong Starter, played it, relaunched, tapped UpdateSmokeBrokenProject, and completed.

  • Ran the smoke test against PR 5202 as an example PR without the release crash fix.
    The test failed at launch with the expected Room integrity failure:
    IllegalStateException: Pre-packaged database has an invalid schema: project_response
    This confirms the workflow catches the legacy update crash scenario before the fix is merged.
    Log shows: Room cannot verify the data integrity... Expected identity hash: 2d6dd..., found: c60e...

Your checklist for this pull request

Please review the contributing guidelines and wiki pages of this repository.

  • Include the name of the Jira ticket in the PR’s title
  • Include a summary of the changes plus the relevant context
  • Choose the proper base branch (develop)
  • Confirm that the changes follow the project’s coding guidelines
  • Verify that the changes generate no compiler or linter warnings
  • Perform a self-review of the changes
  • Verify to commit no other files than the intentionally changed ones
  • Include reasonable and readable tests verifying the added or changed behavior
  • Confirm that new and existing unit tests pass locally
  • Check that the commits’ message style matches the project’s guideline
  • Stick to the project’s gitflow workflow
  • Verify that your changes do not have any conflicts with the base branch
  • After the PR, verify that all CI checks have passed
  • Post a message in the catroid-stage or catroid-ide Slack channel and ask for a code reviewer

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds a GitHub Actions “update smoke” workflow to exercise a real-world upgrade path (install baseline APK → seed on-device app data/legacy DB → update in place → launch and perform basic UI interactions) to catch regressions that only occur on upgraded installs.

Changes:

  • Add an updateTest release-like build type and allow overriding versionCode via -PversionCodeOverride.
  • Add an update-smoke automation script that seeds fixtures, performs an in-place update, and fails on crash/ANR signals with artifact collection.
  • Add a new GitHub Actions workflow to build baseline/PR APKs, run the emulator-based update smoke, and upload diagnostics.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
catroid/build.gradle Adds versionCode override support and introduces updateTest build type used by the workflow.
automationScripts/update-smoke/run-update-smoke.sh New end-to-end update test script (install baseline, seed storage/DB, update, launch, UI checks, log capture).
.github/workflows/update_smoke.yml New CI workflow wiring builds, caching, emulator run, and artifact upload.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread automationScripts/update-smoke/run-update-smoke.sh
Comment thread .github/workflows/update_smoke.yml
@wslany

wslany commented Apr 25, 2026

Copy link
Copy Markdown
Member Author

Addressed Copilot's two update-smoke review comments:

  1. Logcat false positives after intentional am force-stop

    • Fixed in 3d10a33ee by clearing logcat immediately after the intentional force-stop and before relaunching the app.
    • I kept the process-death / SIG: 9 checks because those caught the actual Catroid update crash via BaseExceptionHandler; clearing logcat around the deliberate stop avoids the false positive while preserving the real crash signal.
  2. Baseline APK cache key becoming stale for branch refs

    • Fixed in 3d10a33ee by resolving the checked-out baseline commit and keying the cache with that SHA instead of the ref name.
    • Follow-up d946bc004 also changes the baseline semantics to match the intended release/update scenario: normal PR runs now install the Play Store baseline branch (master for now, later main) rather than the PR target branch. Manual runs can still override BASE_REF.
    • Added a push trigger on develop so, after this workflow is merged, the baseline APK cache can be warmed on the base branch and reused by later PRs until the Play Store baseline branch moves.

@wslany

wslany commented Apr 26, 2026

Copy link
Copy Markdown
Member Author

I asked Claude Code 4.7 for a review of the PR until here and it wrote the following:

Review: [Catroid#5205 – CATROID-1655 Add update smoke CI workflow](#5205)

Status: Draft · author wslany · 3 files, +419/-1 · base develop

Overall

The intent is solid and well-motivated: catch upgrade-only regressions (Room schema migrations, storage layout changes, broken legacy projects) by installing a real Play Store baseline, seeding it with fixtures, then upgrading to the PR APK. The PR even cites a real catch (the schema crash in PR #5202), which is the best argument for merging this kind of harness. Below is what I'd push back on before un-drafting.

catroid/build.gradle

  • defaultVersionCode = ... : 99 — make sure the fallback (99) matches the current versionCode baseline rather than a hard-coded constant. If the file previously used a computed value (e.g. derived from git or a shared constant), replacing it with 99 will silently downgrade Play-track builds. Show the diff against the existing assignment, not just the new ternary.
  • updateTest { initWith buildTypes.release; signingConfig = signingConfigs.debug }release typically has minifyEnabled true + R8/Proguard. Inheriting that is exactly what you want (the smoke test should exercise the shipped shrinker config), but pair it with testCoverageEnabled false and confirm proguard-rules.pro is applied. Also: a release-shaped APK signed with the debug key cannot be installed as an update over a Play-signed baseline — signatures must match. The CI works because both sides are built locally with debug signing; just document that this build type is not suitable for upgrading over an actually-published APK, only over a locally-rebuilt baseline.
  • matchingFallbacks = ['release'] is correct for resolving dependency variants.

.github/workflows/update_smoke.yml

  • Cache key staleness — author said this was addressed; verify the key includes the resolved baseline commit SHA, not just the branch name, otherwise a force-push to the Play Store branch yields a stale APK. Suggest key: baseline-apk-${{ steps.resolve.outputs.sha }}-${{ hashFiles('**/*.gradle*', 'gradle/**') }}.
  • versionCodeOverride: 100001 is fine as a sentinel, but if the project ever ships a real version code ≥ 100001 the test becomes a no-op (downgrade attempt). Compute it as baselineVersionCode + 1 instead of hard-coding.
  • API 35 emulator on every PR is slow (~10–15 min boot + test). Confirm this is on pull_request only for paths that touch catroid/src/**, migrations, or build.gradle — otherwise it's a tax on doc-only PRs. A paths: filter or workflow_dispatch-only default is reasonable for a new harness still proving itself.
  • 60 min timeout is generous; once stable, tighten to ~25 min so a hung emulator fails fast.
  • Artifact upload if: always() — good, keep it.
  • Java 21 + Gradle cache — fine.

automationScripts/update-smoke/run-update-smoke.sh

A few things I'd want to see before this is non-draft:

  • set -euo pipefail at the top, plus trap to dump logcat + uiautomator dump on any failure, not just at the end.
  • Logcat false-positives — author mentioned addressing this. Make sure the crash detector greps for FATAL EXCEPTION/ANR in <pkg>/Process .* org.catrobat.catroid .* died, scoped to the package (adb logcat --pid=$(adb shell pidof -s org.catrobat.catroid) or -T since boot). Free-text greps for "Exception" will match benign library logs and turn the job red.
  • Fixture data path — pushing into /data/data/org.catrobat.catroid/... requires run-as and cat > redirection, not adb push directly (push lands in /sdcard). If you're using adb push to /sdcard/Android/data/... that's fine on API ≤ 29 but scoped storage on API 35 makes that read-only for other apps. Worth re-checking on the actual API 35 image.
  • Legacy DB fixture — please commit the SQLite file (or the SQL to generate it) into the repo with a comment noting which historical schema version it represents. Otherwise the test silently rots when someone bumps Room without updating the fixture.
  • "Deliberately broken project" — good idea, but assert the expected outcome (graceful "project corrupted" toast / project skipped), not just "no crash." Otherwise the app could regress to silently swallowing the broken project and the test would still pass.
  • UI interactions via input tap coordinates are extremely fragile across screen sizes and locales. Prefer uiautomator with resource-IDs, or skip the UI step and rely on am start + logcat.
  • Strict mode env varSTRICT_UI=1 should be the CI default; an opt-out lenient mode is fine for local dev.

Things missing / would strengthen the PR

  • A negative-control commit in the PR description (a reverted reproducer) showing the workflow goes red on a known-bad change. The PR CATROID-1653 Align DST generator fixtures #5202 mention is good; a 2-line "how to reproduce locally" snippet in the script header is even better.
  • Docs: a short automationScripts/update-smoke/README.md covering env vars, how to run locally with two pre-built APKs, and the signing-mismatch caveat above.
  • Concurrency group in the workflow (concurrency: { group: update-smoke-${{ github.ref }}, cancel-in-progress: true }) so push-spam doesn't queue 5 emulators.

Verdict

Approach is right and the catch in #5202 justifies the harness. Block on: signing/upgrade caveat documented, version-code override computed not hard-coded, logcat detector scoped to the app PID, fixture DB committed with schema version, and a paths: filter so this doesn't run on every doc-only PR. Nothing here is structural — all addressable before exiting draft.

@wslany
wslany force-pushed the codex/add-update-smoke-ci branch from 6ac8a8d to 09ee134 Compare April 26, 2026 07:03
@wslany

wslany commented Apr 26, 2026

Copy link
Copy Markdown
Member Author

I asked Claude Code 4.7 for a review of the PR until here and it wrote the following:

Review: [Catroid#5205 – CATROID-1655 Add update smoke CI workflow](#5205)

Status: Draft · author wslany · 3 files, +419/-1 · base develop
[...]

Thanks, I addressed the actionable points from this review and then rewrote the draft branch into a cleaner two-commit history.

Changes made:

  • PR APK version code is now computed as baselineVersionCode + 1 from the baseline APK metadata.
  • Baseline APK cache stores APK + metadata and is keyed by resolved Play Store baseline commit SHA plus Gradle/build-file hash.
  • Added paths-ignore for docs/Markdown-only changes.
  • Added workflow concurrency and reduced timeout to 35 minutes.
  • Moved the legacy Room v2 DB SQL into automationScripts/update-smoke/fixtures/legacy-room-v2-app-database.sql, with comments documenting schema version and identity hash.
  • Added automationScripts/update-smoke/README.md with local run instructions, env vars, seeded fixtures, rootable-emulator requirement, and the debug-signing caveat.
  • Added failure artifact capture on non-zero script exit.
  • Explicitly disabled Android test coverage for the updateTest build type and documented why debug signing is used.

A few notes:

  • set -euo pipefail and strict UI mode were already enabled.
  • Direct /data/data/... seeding is intentional; the test uses a rootable emulator with adb root.
  • The defaultVersionCode fallback remains the existing develop value. Only -PversionCodeOverride changes the version code for updateTest.

The branch now has two commits:

  • [task] add update smoke workflow
  • [task] document update smoke workflow

@wslany
wslany force-pushed the codex/add-update-smoke-ci branch 4 times, most recently from b20a3d0 to d1a6896 Compare April 26, 2026 08:15
@wslany

wslany commented Apr 26, 2026

Copy link
Copy Markdown
Member Author

Note on the current Catroid Update Smoke Test failure:
image
and
image

This failure is expected on this PR as it stands. The workflow now successfully builds the baseline APK from master, builds the PR APK, starts the emulator, installs the baseline, seeds the legacy/broken project state, updates to the PR APK, and launches Catroid.

The failing part is the actual smoke assertion after launch:

BaseExceptionHandler: uncaughtException

That is the intended signal for this draft branch, because this PR adds the update-smoke test but does not include the release crash fix itself. Once this workflow is run on top of the fix branch, this check should turn green (I have tested this, and it is green).

@wslany
wslany marked this pull request as ready for review April 26, 2026 09:04
@wslany wslany added the Active Member Tickets that are assigned to members that are still currently active label Apr 26, 2026
Comment thread .github/workflows/update_smoke.yml Outdated
Comment thread .github/workflows/update_smoke.yml Outdated
Comment thread .github/workflows/update_smoke.yml Outdated
@wslany
wslany force-pushed the codex/add-update-smoke-ci branch from d1a6896 to b85d86a Compare April 28, 2026 12:17
@wslany
wslany requested a review from ratschillerp April 28, 2026 12:31
@wslany
wslany force-pushed the codex/add-update-smoke-ci branch from b85d86a to 5c7a6eb Compare April 28, 2026 12:33
@sonarqubecloud

Copy link
Copy Markdown

@wslany

wslany commented Apr 28, 2026

Copy link
Copy Markdown
Member Author

After #5203 was merged into develop (plus some chores), this test is now green:
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Active Member Tickets that are assigned to members that are still currently active

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants