Repository navigation
Add a GitHub Actions CI workflow - #173
Merged
Merged
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Date parsing became less strict, and one synchronization timeout can still allow a false-positive test.
Review effort: Balanced
Findings: 2
Open (2)
What changed in this PR
Adds GitHub Actions CI and compatibility fixes required for successful Android builds and tests.
Changes:
- Adds parallel lint, test, screenshot, and release-build jobs.
- Creates placeholder CI configuration and disables Crashlytics mapping uploads in CI.
- Fixes date formatting, locale observation, and asynchronous tests.
| File | Description |
|---|---|
.github/workflows/ci.yml |
Defines CI jobs, triggers, artifacts, and concurrency. |
.github/actions/setup/action.yml |
Configures Java, Gradle, and placeholder credentials. |
app/build.gradle |
Disables CI Crashlytics mapping uploads. |
DateFormatterUtil.kt |
Replaces API-26 date handling. |
RateMediaSheet.kt |
Uses Compose’s observable locale. |
MyListsViewModelTest.kt |
Improves asynchronous state synchronization. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
webianks
force-pushed
the
webianks/ci
branch
3 times, most recently
from
October 4, 2026 15:49
ab04336 to
3281018
Compare
salRoid
previously approved these changes
Oct 4, 2026
Base automatically changed from
webianks/content-v4-improvements
to
development
October 4, 2026 16:50
- formatReleaseDate used java.time, which needs API 26 or core library desugaring while minSdk is 24 (NewApi), and silently returned an empty string on Android 7. Use SimpleDateFormat instead. - The rating sheet read Locale.getDefault() inside a composable (NonObservableLocale). - MyListsViewModelTest synchronised on getLists() being called rather than on its result reaching the StateFlow, so a late load could undo the optimistic delete under test.
Runs on pull requests and on pushes to development and release/**, as three parallel jobs: release lint, unit and screenshot tests, and the release bundle (the build that exercises R8). secrets.properties and google-services.json are gitignored, so a shared setup action writes placeholders; no repository secrets are needed, which keeps CI working for pull requests from forks. With a placeholder Firebase project there is nowhere to upload the R8 mapping file, so that upload is skipped when CI is set.
Review feedback: - formatReleaseDate accepted variable-width fields such as 1999-1-5 or 99-10-15, which LocalDate.parse used to reject. Check the yyyy-MM-dd shape before parsing and cover the malformed cases. - waitFor in MyListsViewModelTest returned normally on timeout, so a test could carry on from a state that never arrived and still pass. It now fails on timeout. Races seen on the GitHub runner, where work on a real Dispatchers.IO thread finished before an intermediate state was asserted: - LoginViewModelTest: logout's cleanup could finish before the collector saw isLoggingOut go true, so the StateFlow conflated true -> false and never emitted. - MyListsViewModelTest: the rollback could land before the optimistic removal was asserted. The background step is now held on a latch until the intermediate state has been asserted. Also upload the rendered screenshots when the test job fails, so differences between the macOS-generated references and the Linux runner can be inspected.
The teardowns called viewModelScope.cancel(), which returns while coroutines on real threads are still winding down. On the GitHub runner one from a previous test class touched Dispatchers.Main after MainDispatcherRule had reset it, and the failure was reported against the next test to start (ShowsViewModelTest). Cancel and join instead, in all 15 teardowns that shared the pattern.
23 of the 48 references, generated on macOS, differed from the Linux CI runner's renders by at most 2 levels per channel - invisible, but spread over up to 24% of an image's pixels, so a pixel-count threshold could not absorb it without also hiding real regressions. Replace those 23 with the runner's own renders so CI compares like with like. Validating locally on macOS will now report those screens as different.
Renders the references on the Linux runner and uploads them as an artifact, to be committed after an intended UI change.
webianks
dismissed
salRoid’s stale review
October 4, 2026 16:50
The merge-base changed after approval.
salRoid
previously approved these changes
Oct 4, 2026
The GitHub runner failed two more tests of the same kind as before, where a state is asserted while work on a real Dispatchers.IO thread can already have moved past it: - MovieDetailsViewModelTest: toggleListMembership's rollback could land before the optimistic flip was asserted. - FranchiseViewModelTest: the failing fetch could finish before the collector saw isLoading go true, so the StateFlow conflated true -> false. A sweep of the suite found the same shape in LoginViewModelTest (getRequestToken failure) and CollectionsViewModelTest (trySync), which had not failed yet. All four now hold the background step until the intermediate state has been asserted. MovieDetailsViewModelTest's waitFor also fails on timeout now.
salRoid
approved these changes
Oct 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add a GitHub Actions CI workflow
Adds CI for the project, plus the small fixes needed for it to pass.
What runs
On every pull request, and on pushes to
developmentandrelease/**, three jobs run in parallel:./gradlew :app:lintRelease./gradlew :app:testDebugUnitTestthen:app:validateDebugScreenshotTest./gradlew :app:bundleReleaseReports are uploaded as artifacts when a job fails. A newer push to the same branch cancels the run in progress.
How it builds without secrets
secrets.propertiesandgoogle-services.jsonare gitignored, and the build reads both at configuration time. A shared setup action (.github/actions/setup) writes placeholder versions of both, so:Nothing built in CI is shipped or calls a real API. Because the placeholder Firebase project has nowhere to receive the R8 mapping file,
app/build.gradleskips the Crashlytics mapping upload when theCIenvironment variable is set. Local release builds are unchanged.Fixes included so CI passes
formatReleaseDateusedjava.time, which needs API 26 whileminSdkis 24. Lint flagged it (NewApi), and on Android 7 it silently returned an empty date. It now usesSimpleDateFormat.Locale.getDefault()inside a composable (NonObservableLocale). It now usesLocalLocale.MyListsViewModelTesthad a race: it waited for the load call to start instead of for its result, so a late load could undo the delete under test. It now waits for the loaded state.Testing
All four commands were run locally in a clean checkout using only the placeholder config and
CI=true, and all passed. This pull request is the first run on GitHub's runners, so these are not yet confirmed there: