more test coverage - #6
Merged
Merged
Conversation
Adds real-Core behavior coverage for two high-value targets that the existing unit tests leave under-covered: - MatchProcessorBehaviorTest (new): locks in the contract of the single public entry point generateResultsOfMatch — unpredictable short-circuit, predictable path populates VehicleState predictions, and Prediction rows reach the DB via the async DataDbLogger. - AvlProcessorBehaviorTest: adds the TRIP_ID assignment path (existing suite only covered BLOCK_ID), cacheAvlReportWithoutProcessing, makeVehicleUnpredictable's unwind on a predictable vehicle, and the sched-based-preds guard against updating lastRegularReportProcessed. Introduces pinClockToHappyPathAnchor so multiple happy-path tests can coexist without tripping setLastAvlReport's "only store newer" guard under JUnit's nondeterministic ordering. No production code changes.
An operator who enables the default holding-time generator via transitclock.core.holdingTimeGeneratorClass but does not configure transitclock.holding.controlStops would previously hit a NullPointerException on the first arrival event: StringListConfigValue.getValue() returns null when unset, and getControlPointStops iterated that null without a guard. isControlStop unconditionally calls getControlPointStops, so the NPE is reachable from every public generateHoldingTime entry point. Treats an unset control-stop list as "no control stops" — the feature correctly becomes a no-op in that case, matching the safety contract. Surfaced while adding HoldingTimeGeneratorBehaviorTest, which locks in three invariants against real Core: - departure events never produce a holding time - arrivals at non-control stops never produce a holding time - getControlPointStops() returns an empty (non-null) list by default
Convert the old <b>-tag headers to markdown, update the project description to reflect the current fork lineage, and inline the `mvn test` / QuickStart MDEP-187 explanation previously delegated to CLAUDE.md.
- Warn once at HoldingTimeGeneratorDefaultImpl construction when transitclock.holding.controlStops is unset so operators see the feature is inactive instead of silently producing zero holding times. - Drop the now-redundant null-guard in isControlStop and repair the sibling NPE in predictionsToLongArray that dereferenced predictions before its null check. - Assert Prediction rows stay out of the DB on the unpredictable-vehicle short-circuit test so the guard regression would be caught if the predictable check were ever moved. - Trim block comments in the pipeline tests that restated method names or assertion messages; keep the monotonic-clock, feed-up-monitoring, and clear-predictions-before-reinvoke rationales.
📝 WalkthroughWalkthroughThis PR updates documentation in README.md, improves null-safety in HoldingTimeGeneratorDefaultImpl with defensive checks, and significantly expands the pipeline behavior test suite with new test classes (HoldingTimeGeneratorBehaviorTest, MatchProcessorBehaviorTest) and refactored AvlProcessorBehaviorTest with centralized test helpers. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
README.md (1)
8-60: Consider standardizing heading style for consistency.The README mixes setext-style headings (underlined with
====) and atx-style headings (##). While both are valid Markdown, using a consistent style throughout improves readability and passes linter checks.♻️ Proposed fix: Convert all section headings to atx style
-==== +# TheTransitClock -==== -## About this Repo +## About this Repo -## Build +## Build -## Setup +## Setup -## Running tests +## Running tests -### Code coverage +### Code coverage🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 8 - 60, The README mixes setext-style underlined headings and atx-style (##) headings; standardize by converting all setext headings (e.g., "About this Repo", "Build", "Setup", "Running tests", "Code coverage") to consistent atx-style headings (use ## for top-level sections and ###/#### for subsections like "Pipeline tests" and "Integration tests"), updating each heading line to start with the appropriate number of # characters and removing the underline lines so all headings follow the same atx format.transitclockPipelineTests/src/test/java/org/transitclock/pipelinetests/MatchProcessorBehaviorTest.java (2)
72-79: Consider migrating from deprecated Hibernate Criteria API.
session.createCriteria()is deprecated in Hibernate 5+. While functional for tests, consider using JPA CriteriaBuilder or HQL for better long-term compatibility.♻️ Optional: Use HQL instead
`@SuppressWarnings`("unchecked") private static List<Prediction> queryPredictionsForVehicle(String vehicleId) { try (Session session = HibernateUtils.getSession(AgencyConfig.getAgencyId())) { - return session.createCriteria(Prediction.class) - .add(Restrictions.eq("vehicleId", vehicleId)) - .list(); + return session.createQuery( + "FROM Prediction p WHERE p.vehicleId = :vehicleId", Prediction.class) + .setParameter("vehicleId", vehicleId) + .list(); } }🤖 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/MatchProcessorBehaviorTest.java` around lines 72 - 79, The test helper method queryPredictionsForVehicle uses the deprecated session.createCriteria API; replace it with a non-deprecated query mechanism (e.g., HQL or JPA CriteriaBuilder) inside the same try-with-resources block: build a typed query against the Prediction entity (for example using session.createQuery("from Prediction where vehicleId = :vid", Prediction.class) or equivalent CriteriaBuilder usage), bind the vehicleId parameter, execute the query to get the List<Prediction>, and remove the Restrictions.eq call; keep the method signature and HibernateUtils.getSession(AgencyConfig.getAgencyId()) usage intact.
81-96: Polling approach is functional; Awaitility could simplify.The manual polling loop with timeout is correct and handles interruption properly. If the project uses Awaitility elsewhere, it would provide cleaner async assertions. Otherwise, this is fine as-is.
🤖 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/MatchProcessorBehaviorTest.java` around lines 81 - 96, The manual polling loop in waitForDbRows (using DB_FLUSH_TIMEOUT_MS and DB_POLL_INTERVAL_MS, calling fetcher.get()) can be replaced with Awaitility to simplify async waiting: use Awaitility.await().atMost(Duration.ofMillis(DB_FLUSH_TIMEOUT_MS)).pollInterval(Duration.ofMillis(DB_POLL_INTERVAL_MS)).untilAsserted(() -> assertThat(fetcher.get().size()).isGreaterThanOrEqualTo(minRows)); update imports accordingly and remove the Thread.sleep/InterruptedException handling inside waitForDbRows; keep the method signature and semantics (return the final fetcher.get() result after the await) so callers (tests) behave the same.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Line 12: Update the README sentence to hyphenate the compound adjective:
change the phrase "cost effective system" to "cost-effective system" in the line
containing "By providing a complete open-source system, agencies can have a cost
effective system and have full ownership of it." so the compound adjective is
grammatically correct.
---
Nitpick comments:
In `@README.md`:
- Around line 8-60: The README mixes setext-style underlined headings and
atx-style (##) headings; standardize by converting all setext headings (e.g.,
"About this Repo", "Build", "Setup", "Running tests", "Code coverage") to
consistent atx-style headings (use ## for top-level sections and ###/#### for
subsections like "Pipeline tests" and "Integration tests"), updating each
heading line to start with the appropriate number of # characters and removing
the underline lines so all headings follow the same atx format.
In
`@transitclockPipelineTests/src/test/java/org/transitclock/pipelinetests/MatchProcessorBehaviorTest.java`:
- Around line 72-79: The test helper method queryPredictionsForVehicle uses the
deprecated session.createCriteria API; replace it with a non-deprecated query
mechanism (e.g., HQL or JPA CriteriaBuilder) inside the same try-with-resources
block: build a typed query against the Prediction entity (for example using
session.createQuery("from Prediction where vehicleId = :vid", Prediction.class)
or equivalent CriteriaBuilder usage), bind the vehicleId parameter, execute the
query to get the List<Prediction>, and remove the Restrictions.eq call; keep the
method signature and HibernateUtils.getSession(AgencyConfig.getAgencyId()) usage
intact.
- Around line 81-96: The manual polling loop in waitForDbRows (using
DB_FLUSH_TIMEOUT_MS and DB_POLL_INTERVAL_MS, calling fetcher.get()) can be
replaced with Awaitility to simplify async waiting: use
Awaitility.await().atMost(Duration.ofMillis(DB_FLUSH_TIMEOUT_MS)).pollInterval(Duration.ofMillis(DB_POLL_INTERVAL_MS)).untilAsserted(()
-> assertThat(fetcher.get().size()).isGreaterThanOrEqualTo(minRows)); update
imports accordingly and remove the Thread.sleep/InterruptedException handling
inside waitForDbRows; keep the method signature and semantics (return the final
fetcher.get() result after the await) so callers (tests) behave the same.
🪄 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: eb11880e-960d-436a-b37f-58b34ef0065d
📒 Files selected for processing (5)
README.mdtransitclock/src/main/java/org/transitclock/core/holdingmethod/HoldingTimeGeneratorDefaultImpl.javatransitclockPipelineTests/src/test/java/org/transitclock/pipelinetests/AvlProcessorBehaviorTest.javatransitclockPipelineTests/src/test/java/org/transitclock/pipelinetests/HoldingTimeGeneratorBehaviorTest.javatransitclockPipelineTests/src/test/java/org/transitclock/pipelinetests/MatchProcessorBehaviorTest.java
|
|
||
| The complete core Java software for the Transitime real-time transit information project. The purpose of the software is to use any type of real-time GPS data to generate useful public transportation information, namely a GTFS-RT Trip Updates feed. | ||
|
|
||
| The system is for both letting passengers know the status of their vehicles and helping agencies more effectively manage their systems. By providing a complete open-source system, agencies can have a cost effective system and have full ownership of it. |
There was a problem hiding this comment.
Fix grammar: hyphenate compound adjective.
The phrase "cost effective" should be hyphenated when used as a compound adjective before a noun.
📝 Proposed fix
-The system is for both letting passengers know the status of their vehicles and helping agencies more effectively manage their systems. By providing a complete open-source system, agencies can have a cost effective system and have full ownership of it.
+The system is for both letting passengers know the status of their vehicles and helping agencies more effectively manage their systems. By providing a complete open-source system, agencies can have a cost-effective system and have full ownership of it. 📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| The system is for both letting passengers know the status of their vehicles and helping agencies more effectively manage their systems. By providing a complete open-source system, agencies can have a cost effective system and have full ownership of it. | |
| The system is for both letting passengers know the status of their vehicles and helping agencies more effectively manage their systems. By providing a complete open-source system, agencies can have a cost-effective system and have full ownership of it. |
🧰 Tools
🪛 LanguageTool
[grammar] ~12-~12: Use a hyphen to join words.
Context: ...-source system, agencies can have a cost effective system and have full ownership...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` at line 12, Update the README sentence to hyphenate the compound
adjective: change the phrase "cost effective system" to "cost-effective system"
in the line containing "By providing a complete open-source system, agencies can
have a cost effective system and have full ownership of it." so the compound
adjective is grammatically correct.
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 by CodeRabbit
Documentation
Bug Fixes