Tests - #1
Merged
Merged
Conversation
Aligns the transitclockQuickStart module's Eclipse compiler compliance/source/target with the Java 17 build (5d3ed0b); the settings were still pinned to 1.7.
Documents the Maven multi-module layout, the Core → API (RMI) → Webapp runtime pipeline, the shaded main classes under org.transitclock.applications, and repo-specific gotchas (config rev vs. travel-times rev, org.transitclock vs. legacy org.transitime naming) so future Claude Code sessions can orient without re-deriving it.
transitclockQuickStart's dependency:copy goal needs the API WAR, which isn't produced by the test phase, so `mvn test` on the full reactor fails. Recommend `mvn verify` instead and note the single-module and exclude-QuickStart escape hatches.
The QuickStart test Hibernate config has carried raw <<<<<<< / ======= / >>>>>>> conflict markers since the original transitclock/transitime merge (d8989115, e600fd30). That made the XML unparseable, so every QuickStart test that loads it (four of five) failed with "ConfigurationException: Unable to perform unmarshalling at line number 0 and column 0". Kept the HSQL side of the conflict — the file is named hsql_hibernate.cfg.xml and the other side was postgres-targeted.
Two QuickStart tests cannot pass against current production code: - TransiTimeTest drives GtfsFileProcessor, which calls System.exit(-1) from ~35 sites across org.transitclock.gtfs.* when it hits any processing error. That tears down the surefire fork before the test's try/catch can turn it into a normal failure. test_2_Core additionally hardcodes a Windows config path (C:\Users\Brendan\...). - ApiJettyTest hardcodes port 8080, never stops Jetty (no @Before/@after), and asserts nothing meaningful. It fails on any host where 8080 is already bound. @ignore'd with explanations rather than deleted so the intent is preserved for whoever picks up the QuickStart smoke-test suite. The remaining QuickStart tests (AppTest, CreateApiTest, createWebAgencyTest) now pass and mvn verify goes green across the reactor.
The shade plugin regenerates this file every package; it was showing up as untracked after every build.
…asAdjuster Establish a baseline of fast pure-Java unit tests for the highest-leverage utility and value-type classes. Tests are pinned to current behavior — including the quirks (e.g. MathUtils.max() seeded at 0, the non-arg TemporalDifference.isWithinBounds() using strict <) — so future edits get flagged rather than silently shifting. 97 tests across 7 files; mvn -pl transitclock test BUILD SUCCESS in ~3s.
Adds the JaCoCo plugin at the root aggregator so any module declaring <parent> picks up instrumentation and per-module HTML reports at target/site/jacoco/index.html during mvn verify. No coverage thresholds yet — this is a baseline. Note: transitclockApi and transitclockWebapp do not declare <parent>, so they won't emit reports without further changes.
New coverage-report module runs jacoco:report-aggregate at the verify
phase, producing a single HTML report at
coverage-report/target/site/jacoco-aggregate/index.html that merges
transitclockCore, transitclockBarefootClient, and transitclockTraccarClient.
Skip JaCoCo in transitclockQuickStart: it bundles api.war and web.war
into target/classes as resources, and JaCoCo's analyzer chokes on the
multi-release JARs inside the WARs ("Can't add different class with
same name"). QuickStart is a launcher, not something worth measuring
coverage on, so it's excluded from the aggregate as well.
Adds a short Code Coverage section to the README and a more detailed one to CLAUDE.md, covering per-module reports, the aggregate in coverage-report/, why QuickStart is skipped, and which modules don't inherit the plugin.
…uleTime These db.structs classes are mostly pure-Java value types with small amounts of math and bookkeeping — they don't need a DB or Hibernate context to construct. The tests lock down: - Location: equals/hashCode on lat/lon, distance delegations, toString. - Vector: length, distance and match-distance pass-throughs, cardinal heading() and angle() conventions, beginning/end/middle/locAlongVector slicing, zero-length handling. - VectorWithHeading: 0-360 normalization (which plain Vector does not do) and headingOK delegation. - Extent: ±infinity initial bounds, add(Location)/add(Extent) expansion, equals/hashCode, and isWithinDistance with a small meter buffer. - ScheduleTime: getTime() precedence (departure over arrival, fallback to arrival, null when both missing) and toString field-presence behavior. 39 new tests; mvn -pl transitclock test BUILD SUCCESS. Per-class coverage ranges from 73% (VectorWithHeading) to 93% (Location); aggregate still reads 2% because these classes are small relative to the 142k-instruction transitclockCore bundle.
Extend db.structs coverage into entity-shaped classes that are mostly testable without a Hibernate session or live Core. - AvlReport (24 tests): constructor variants (lat/lon vs Location, NaN speed/heading becoming "invalid", source truncated to SOURCE_LENGTH, no-speed overload), getHeading's low-speed NaN rule, latency zero when unprocessed, AssignmentType predicates (block-type covers both regular and sched-based variants), setAssignment null-guard, and the validateData happy/unhappy paths. setTimeProcessed requires Core, so the positive-latency path is left alone with a comment. - Headway (4 tests): Headway's public constructor pulls configRev from Core, so the tests go through the no-arg Hibernate constructor plus setters. Equality only considers headway + five id fields — timing and stats don't participate. - Calendar (8 tests): GTFS-style "1" means true and everything else (including nulls and "yes"/"true") is false; trimmed " 1 " still counts. End date is configured-end + 1 day. Round-trip of string formatters and composite-id equals/hashCode. - ActiveRevisions (4 tests): default -1/-1 sentinels, setters, isValid() requires BOTH revisions non-negative, toString. 40 tests, all green. Aggregate coverage for the targeted classes is now: Headway 77%, Calendar 61%, AvlReport 52%, ActiveRevisions 39%.
…Stop Continues filling in structs-package coverage. HoldingTime uses reflection to set the final holdingTime field so getTimeToLeave/leaveStop can be exercised without going through the Core-dependent value constructors. Stop uses GtfsStop's plain-args constructor with a null-file TitleFormatter so no CSV parsing is needed.
These structs take Gtfs* records whose only public constructor reads a CSVRecord, so each test stages a single-row CSV in memory via CSVFormat + CSVParser. Agency's Extent is exercised with an empty route list. FareRule tests pin the "empty-string internal, null-on-read" contract for its composite-id columns and the properRouteId override path.
MonitoringEvent, Prediction, PredictionAccuracy, PredictionForStopPath, TravelTimesForStopPath, TravelTimesForTrip, VehicleState Frequency/CalendarDate use in-memory CSVRecord staging; Match/Prediction/ PredictionAccuracy/MonitoringEvent reach their Hibernate no-arg constructors (protected in same package, private via reflection where needed), with field mutation via reflection to exercise equals/hashCode and Lifecycle intern-on-load. TravelTimesForTrip pins the documented "equals ignores tripCreatedForId" behavior that's load-bearing for the travel-time cache.
Builds the reactor with `mvn -B -ntp verify` on push/PR to develop, main, and master. Uses JDK 17 Temurin and caches Maven artifacts. Publishes the JaCoCo aggregate report always and surefire reports on failure.
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThis PR adds comprehensive test coverage for database structs and utility classes, establishes JaCoCo code coverage aggregation through a new Maven module, configures CI/CD via GitHub Actions, and provides build/test documentation. Additionally, it updates Eclipse project settings to Java 17 and marks certain integration tests as ignored. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 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 |
This was referenced Apr 22, 2026
Merged
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 by CodeRabbit
Release Notes
New Features
Documentation
Tests
Build