Pipeline behavior tests: predictions, arrivals/departures, matching, block assignment - #4
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughAdds four pipeline-level integration test suites for matching, block assignment, arrival/departure event generation, and prediction generation, plus an Ehcache v3 test configuration used by those tests. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
…ching, and block assignment Extends the transitclockPipelineTests suite with four new behavior test classes covering previously-unobserved pipeline outputs. The existing AvlProcessorBehaviorTest only verified that a matched vehicle became predictable — regressions in the downstream generators would leave that check green while silently breaking user-visible output. - PredictionGeneratorBehaviorTest (6 tests): predictions appear in PredictionDataCache and on VehicleState, cover the in-horizon stops of the current trip, carry correct block/route/vehicle metadata, and emit both arrival and departure predictions for mid-trip stops. - ArrivalDepartureBehaviorTest (4 tests): driving a vehicle through two stops generates departure records at the origin and arrival records at the destination in StopArrivalDepartureCache with correct metadata. - BlockAssignerBehaviorTest (8 tests): exercises BLOCK_ID / TRIP_ID / TRIP_SHORT_NAME / ROUTE_ID / UNSET assignments plus negative cases (invalid block id, invalid trip id) against the real DbConfig. - MatchingBehaviorTest (6 tests): inspects TemporalMatch content — at-stop indicator, stop path index, trip/block/route references, distance to segment under the configured tolerance, populated schedule adherence, and projected-location proximity to the reported AVL point. Pipeline tests now total 35 (up from 11). Also adds a pipeline-test-only ehcache.xml that shadows the main-classpath copy to redirect the ehcache persistence directory from /usr/local/transitclock/cache (not writable in CI) to target/ehcache-pipeline-store.
aaronbrethorst
force-pushed
the
pipeline-tests-2
branch
from
April 23, 2026 02:47
a74dfc0 to
9ea7225
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@transitclockPipelineTests/src/test/java/org/transitclock/pipelinetests/PredictionGeneratorBehaviorTest.java`:
- Around line 128-131: The current assertion using
state.getPredictions().allSatisfy(...) can pass vacuously for an empty list; add
an explicit non-empty precondition before the allSatisfy check (e.g.,
assertThat(state.getPredictions()).isNotEmpty()) so the test fails when
predictions are missing, and apply the same change to the other occurrence that
checks predictionTime vs avlTime (the block using state.getPredictions(),
allSatisfy, getPredictionTime and avlTime).
- Around line 178-191: The test
bothArrivalAndDeparturePredictionsArePresentForMidTripStops currently only
filters predictions by TRIP_ID; update the two checks (the anyArrival and
anyDeparture predicates over state.getPredictions()) to also filter by STOP_MID
so they assert presence of both arrival and departure predictions specifically
for the mid-trip stop. Keep the existing use of IpcPrediction::isArrival (and
its negation) but add a predicate p -> STOP_MID.equals(p.getStopId()) alongside
TRIP_ID.equals(p.getTripId()) to narrow the assertions to the intended stop.
In `@transitclockPipelineTests/src/test/resources/ehcache.xml`:
- Around line 41-42: The test cache configuration uses persistent="true" on the
<ehcache:disk> element which causes disk-backed state to be kept between test
runs; change the <ehcache:disk persistent="true" ...> entries used for tests to
non-persistent (set persistent="false" or remove the attribute) so caches are
in-memory only for hermetic tests and do not write to
target/ehcache-pipeline-store; update the three occurrences of the
<ehcache:disk> element (the shown block and the other two similar blocks
referenced) to disable persistence while keeping the same unit/size values.
🪄 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: f23b68b1-999c-4438-b9fc-db238225a140
📒 Files selected for processing (5)
transitclockPipelineTests/src/test/java/org/transitclock/pipelinetests/ArrivalDepartureBehaviorTest.javatransitclockPipelineTests/src/test/java/org/transitclock/pipelinetests/BlockAssignerBehaviorTest.javatransitclockPipelineTests/src/test/java/org/transitclock/pipelinetests/MatchingBehaviorTest.javatransitclockPipelineTests/src/test/java/org/transitclock/pipelinetests/PredictionGeneratorBehaviorTest.javatransitclockPipelineTests/src/test/resources/ehcache.xml
Comment on lines
+178
to
+191
| public void bothArrivalAndDeparturePredictionsArePresentForMidTripStops() { | ||
| pushHappyPathReport("v-preds-arrdep"); | ||
|
|
||
| VehicleState state = VehicleStateManager.getInstance().getVehicleState("v-preds-arrdep"); | ||
| // PredictionGenerator emits both an arrival and a departure prediction | ||
| // for each non-terminal stop. Missing one type would mean the UI | ||
| // shows only "arriving" but not "departing" (or vice versa) — a | ||
| // silent capability regression rather than a crash. | ||
| boolean anyArrival = state.getPredictions().stream() | ||
| .filter(p -> TRIP_ID.equals(p.getTripId())) | ||
| .anyMatch(IpcPrediction::isArrival); | ||
| boolean anyDeparture = state.getPredictions().stream() | ||
| .filter(p -> TRIP_ID.equals(p.getTripId())) | ||
| .anyMatch(p -> !p.isArrival()); |
There was a problem hiding this comment.
The “mid-trip stops” assertion currently checks only trip-level presence.
This can pass even if mid-stop predictions lose one variant. Filter by STOP_MID (and trip) so the test matches its stated behavior.
Suggested scope fix
- boolean anyArrival = state.getPredictions().stream()
- .filter(p -> TRIP_ID.equals(p.getTripId()))
+ boolean anyArrival = state.getPredictions().stream()
+ .filter(p -> TRIP_ID.equals(p.getTripId()))
+ .filter(p -> STOP_MID.equals(p.getStopId()))
.anyMatch(IpcPrediction::isArrival);
boolean anyDeparture = state.getPredictions().stream()
- .filter(p -> TRIP_ID.equals(p.getTripId()))
+ .filter(p -> TRIP_ID.equals(p.getTripId()))
+ .filter(p -> STOP_MID.equals(p.getStopId()))
.anyMatch(p -> !p.isArrival());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@transitclockPipelineTests/src/test/java/org/transitclock/pipelinetests/PredictionGeneratorBehaviorTest.java`
around lines 178 - 191, The test
bothArrivalAndDeparturePredictionsArePresentForMidTripStops currently only
filters predictions by TRIP_ID; update the two checks (the anyArrival and
anyDeparture predicates over state.getPredictions()) to also filter by STOP_MID
so they assert presence of both arrival and departure predictions specifically
for the mid-trip stop. Keep the existing use of IpcPrediction::isArrival (and
its negation) but add a predicate p -> STOP_MID.equals(p.getStopId()) alongside
TRIP_ID.equals(p.getTripId()) to narrow the assertions to the intended stop.
- Add isNotEmpty() preconditions before allSatisfy() calls in PredictionGeneratorBehaviorTest. AssertJ's allSatisfy passes vacuously on empty collections, which would let a regression that stops producing predictions slip through as a green test. - Drop persistent="true" from the three disk-backed caches in the test ehcache.xml (arrivalDeparturesByTrip, arrivalDeparturesByStop, KalmanErrorCache). Test runs should read fresh cache state, not whatever a prior run left in target/ehcache-pipeline-store. - Rename bothArrivalAndDeparturePredictionsArePresentForMidTripStops to ...OnActiveTrip and tighten its comment. The previous name claimed a per-stop check but the test actually filters only by trip; narrowing to STOP_MID (as suggested in review) fails because mid-trip stops without a scheduled departure don't emit a departure prediction, so whether both variants appear is a trip-level invariant rather than a per-stop one.
3 tasks
Merged
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
1 participant
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.
Summary
Extends the pipeline-tests suite with four behavior test classes (24 new tests) that validate previously-unobserved outputs of the prediction pipeline. Pipeline suite now runs 35 tests total (up from 11).
Each class boots a real Core via
CoreHarnessagainst the WMATA 5A GTFS fixture and asserts on observable state after driving AVL reports through the matcher/generator stack.PredictionGeneratorBehaviorTest(6 tests) — verifiesPredictionGeneratorDefaultImplpopulatesPredictionDataCacheandVehicleState.getPredictions()with the in-horizon stops of the current trip, carrying correct block/route/vehicle metadata and both arrival and departure variants.ArrivalDepartureBehaviorTest(4 tests) — drives a vehicle through two stops on block SE-08's first trip and assertsStopArrivalDepartureCacherecords a departure at the origin and an arrival at the destination with correct identifying metadata.BlockAssignerBehaviorTest(8 tests) — exercises BLOCK_ID / TRIP_ID / TRIP_SHORT_NAME / ROUTE_ID / UNSET assignments plus invalid-id negative cases, and covers thegetRouteIdAssignmententry point.MatchingBehaviorTest(6 tests) — inspectsTemporalMatchcontent: at-stop indicator, stop path index, trip/block/route references, distance-to-segment undermaxDistanceFromSegment, populated real-time schedule adherence, and projected-location proximity to the reported AVL point.Infrastructure change
Adds
transitclockPipelineTests/src/test/resources/ehcache.xmlthat shadows the main-classpath copy to redirect ehcache persistence from/usr/local/transitclock/cache(not writable under CI or most dev machines) totarget/ehcache-pipeline-store. Needed the first time pipeline tests causedArrivalDepartureGeneratorDefaultImpl.updateCacheto initializeTripDataHistoryCache.Rationale
AvlProcessorBehaviorTestonly checksvehicle.isPredictable()— a coarse signal. Regressions where the matcher snaps to the wrong segment, predictions carry wrong metadata, or arrival/departure records are silently dropped would all leave that check green while breaking user-visible output. These new tests close those observability gaps by asserting on actual pipeline outputs (cache contents, match fields, record metadata).Test plan
mvn -pl transitclockPipelineTests -am -P include-pipeline-tests test→ 35/35 passingmvn verify(default profile, no pipeline profile) → 580/580 passing, unaffectedreuseForks=false, so adding classes doesn't risk cross-class singleton pollutionSummary by CodeRabbit