Test DefaultArrivalsRepository's stale-fallback/CAS concurrency against the real class (#1909)

[bmander]

Fixes #1909 (from the 2026-07-16 debt review, finding 9).

Problem

The single-write LastGood holder and the fresh-wins-over-stale compareAndSet in DefaultArrivalsRepository were subtle race fixes (#1865) whose reasoning existed only in comments. ArrivalsViewModelTest uses a fake repository; nothing constructed the real class, so a future simplification could re-break the concurrency invisibly.

Why the class wasn't testable, and the fix

The real class couldn't be built on the JVM because of three incidental Android tendrils. Each is now a small, independently-justified seam — behavior unchanged, verified by the full unit suite on both flavors plus an on-device smoke test:

• ArrivalsDisplay — the Android presentation edge (convertArrivals + stop error strings) behind an interface. DefaultArrivalsDisplay holds the Context, which leaves DefaultArrivalsRepository's constructor entirely — completing the quarantine its docstring already claimed. Key discovery: ArrivalInfo already tolerates a null context (the "Activity destroyed" guard), so the test fake builds real display models with real ETA math.
• ElapsedClock (time/) — the injectable ElapsedTime.now() source, the monotonic counterpart of the existing TimeProvider and the class-boundary form of the "pass the now in" rule — so the stale projection is testable with an exact elapsed delta.
• RouteFavorites — the narrow starring surface (interface segregation) of RouteFavoritesRepository, whose full dependency set (Context/analytics/app scope) isn't JVM-constructible. Its getArrivals-path use is nil; the @Singleton impl is @Binds-bound.

The tests (DefaultArrivalsRepositoryTest, 7, all against the real class)

Hand-rolled fakes (house style) over real StopArrivals wire fixtures:

1. Fresh load publishes a lastLoaded() consistent with the displayed trips (real focusedTrips derivation)
2. Empty-window widen loop grows the window (65 → 125 → 185)
3. Stop recorded once per session, not per poll
4. Failure with no prior data → the display error message
5. Stale fallback: ETAs projected forward by exactly the elapsed device time (Use the server clock (currentTime) as the ETA baseline everywhere; audit the app for time-domain mixing #1612), windowEnd unmoved
6. Uncontended CAS: the re-projection observably refreshes the derived map snapshot (a passed arrival drops out of focusedTrips)
7. The interleaved race: the stale path is parked between its lastGood read and its CAS (a gated fake StopDao.userInfo — the same CompletableDeferred gate technique ArrivalsViewModelTest uses), a fresh load publishes, and the released CAS loses — a stale re-projection can never roll back a concurrently-published fresh snapshot.

Verification

• testObaGoogleDebugUnitTest + testObaMaplibreDebugUnitTest green with -PwarningsAsErrors=true; Hilt graph compiles.
• On-device smoke test (Puget Sound stop 1_75403): arrivals render fully through the new wiring — header, route rows with real labels, ETA pills, "Showing trips until…" footnote, Load more.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Improved arrival updates with more accurate elapsed-time handling and stale-data ETA projections.
  • Added clearer separation for arrival display formatting and route favorites services.
  • Failed refreshes now provide user-facing stop error messages while preserving available stale arrival data.

• Bug Fixes

  • Prevented stale arrival data from overwriting newer results during concurrent updates.
  • Improved handling of empty arrival windows and expired projected arrivals.

• Tests

  • Added coverage for refresh behavior, fallback data, ETA projections, favorite usage, and concurrency.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds injectable seams for monotonic time, arrivals display conversion, and route favorites. Refactors DefaultArrivalsRepository to use them and adds JVM tests for fresh loads, stale fallback projection, snapshot consistency, and concurrent CAS behavior.

Changes

Arrivals repository seams and validation

Layer / File(s)	Summary
Injectable contracts and production bindings onebusaway-android/src/main/java/org/onebusaway/android/time/ElapsedClock.kt, onebusaway-android/src/main/java/org/onebusaway/android/ui/arrivals/ArrivalsDisplay.kt, onebusaway-android/src/main/java/org/onebusaway/android/database/oba/RouteFavoritesRepository.kt, onebusaway-android/src/main/java/org/onebusaway/android/app/di/*	Defines ElapsedClock, ArrivalsDisplay, and RouteFavorites contracts, then binds their production implementations through Hilt.
Repository integration onebusaway-android/src/main/java/org/onebusaway/android/ui/arrivals/ArrivalsRepository.kt	Removes direct context and clock calls, injects the new seams, and routes conversion, error messages, receipt times, and stale projection through them.
Fresh and stale behavior tests onebusaway-android/src/test/java/org/onebusaway/android/ui/arrivals/DefaultArrivalsRepositoryTest.kt	Adds fakes and tests for polling windows, stop usage, error handling, elapsed-time ETA projection, snapshot updates, and stale-versus-fresh CAS ordering.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

• OneBusAway/onebusaway-android#1618: Changes arrivals ETA handling around server time and monotonic elapsed time.
• OneBusAway/onebusaway-android#1727: Adds reactive route-favorite APIs used by the favorites seam.
• OneBusAway/onebusaway-android#1746: Modifies DefaultArrivalsRepository.getArrivals timing and stop-error behavior.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and accurately summarizes the main change: real-class tests for DefaultArrivalsRepository's stale-fallback/CAS concurrency.
Linked Issues check	✅ Passed	The PR adds real-class tests for stale fallback, CAS interleaving, lastLoaded consistency, and trip derivation, matching #1909.
Out of Scope Changes check	✅ Passed	The new seams and DI bindings support the repository testability goal and the linked concurrency tests, so no clear out-of-scope changes appear.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch test/arrivals-repository-cas-1909

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@onebusaway-android/src/test/java/org/onebusaway/android/ui/arrivals/DefaultArrivalsRepositoryTest.kt`:
- Around line 395-399: Update ArrivalsViewModel.refresh() to guard the
loaded.value = data assignment with latest-wins generation or cancellation
validation, so results from older overlapping refreshes are discarded before
updating UI state. Preserve the existing lastLoaded() behavior and allow only
the most recent refresh completion to publish loaded data.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 1e2591b2-9c7c-44ab-9848-79ea515a2c9f

📥 Commits

Reviewing files that changed from the base of the PR and between 4e3dcec and f60f58e.

📒 Files selected for processing (7)

• onebusaway-android/src/main/java/org/onebusaway/android/app/di/AppModule.kt
• onebusaway-android/src/main/java/org/onebusaway/android/app/di/RepositoryModule.kt
• onebusaway-android/src/main/java/org/onebusaway/android/database/oba/RouteFavoritesRepository.kt
• onebusaway-android/src/main/java/org/onebusaway/android/time/ElapsedClock.kt
• onebusaway-android/src/main/java/org/onebusaway/android/ui/arrivals/ArrivalsDisplay.kt
• onebusaway-android/src/main/java/org/onebusaway/android/ui/arrivals/ArrivalsRepository.kt
• onebusaway-android/src/test/java/org/onebusaway/android/ui/arrivals/DefaultArrivalsRepositoryTest.kt