Skip to content

Fixup integration tests - #10

Merged
aaronbrethorst merged 8 commits into
developfrom
fixup-integration-tests
Apr 24, 2026
Merged

Fixup integration tests#10
aaronbrethorst merged 8 commits into
developfrom
fixup-integration-tests

Conversation

@aaronbrethorst

@aaronbrethorst aaronbrethorst commented Apr 24, 2026

Copy link
Copy Markdown
Member

Summary by CodeRabbit

  • Documentation

    • Added a comprehensive guide for refreshing integration-test fixtures and warned that prediction outputs are non-deterministic across JVM replays, affecting baseline comparisons and test assertions
  • New Features

    • Added a GTFS subsetting CLI for per-route fixture creation
    • Added a detour-candidate ranking tool to find suitable vehicle traces
  • Tests

    • Re-enabled a detour recovery integration test against refreshed fixtures
    • Updated prediction-accuracy integration fixtures and documented why the baseline test remains disabled

subset_gtfs.py slices a full GTFS feed down to one route with
referential closure (routes → trips → stop_times → stops → shapes →
calendar). find_detour_candidates.py ranks AVL traces by how detour-
like they look by measuring each vehicle's maximum deviation from the
route shape.

Together they support the capture → promote pipeline documented in
integration-tests.md.
WMATA's June 2025 Better Bus Network redesign renamed every route
to a <zone-letter><number> scheme, so the 2016-era S2/3T/5A fixtures
could not simply be re-captured. Replaced with fresh traces from
equivalent current routes:

- S2 (16th St trunk) → D40 "7 St-Georgia Av", vehicle 5506
- 3T (VA crosstown) → A40 "Columbia Pike-National Landing", vehicle 3151
- plus D20 "H Street", vehicle 7198, kept as a spare for future tests

5A fixtures are left in place since their two tests still pass.

RecoverFromDetourTest is un-ignored and passing against A40_3151
(vehicle exits layover, final schedule adherence -7.5 min within the
10-min threshold). PredictionAccuracyIntegrationTest has its path
constants updated but remains @ignored pending regeneration of
pred/D40_5506.csv as its regression baseline.

integration-tests.md documents the end-to-end refresh workflow for
future captures. Fixtures were produced via tools/wmata_capture plus
the new subset_gtfs.py and find_detour_candidates.py helpers.

Refs #7 (detour test, now re-enabled)
Refs #8 (prediction accuracy, baseline pending)
Tried to regenerate pred/D40_5506.csv by dumping the current predictor's
output against the refreshed fixtures, intending to un-ignore
testPredictions. Discovered that PlaybackModule.runTrace is
non-deterministic across JVMs: replaying the same AVL trace produces
prediction counts that vary by >20% (observed 2155, 2696, 2118 across
three runs) and ArrivalDeparture counts that vary by >2x.

A frozen CSV baseline therefore cannot serve as a regression signal —
the test's newTotalError/oldTotalError comparison fails on run-to-run
variance, not on predictor regression. Update the @ignore message to
name the real blocker and extend integration-tests.md with two
suggested paths for re-enabling (deterministic replay, or
tolerance-based assertions).

RecoverFromDetourTest remains live and passing; no regression.

Refs #8
CLAUDE.md: point at integration-tests.md for fixture-refresh work and
record that PlaybackModule.runTrace is non-deterministic — needed so
future test design doesn't assume frozen CSV baselines will work.

tools/wmata_capture/README.md: the promotion recipe still pointed at
2016 S2 paths and didn't know about the GTFS-subsetting step. Rewrite
that section to use subset_gtfs.py and find_detour_candidates.py, drop
the now-obsolete block_id caveat (100% coverage post-BBN redesign),
note that the pred baseline path is currently blocked on
non-determinism, and add the route-ID rename caveat so future captures
know the 2016 short names are gone.
@coderabbitai

coderabbitai Bot commented Apr 24, 2026

Copy link
Copy Markdown

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

Refreshes integration-test fixtures (remove 3T/S2, add A40/D20/D40), adds GTFS helper CLIs for subsetting and detour candidate ranking, updates integration tests to use refreshed fixtures (un-ignore RecoverFromDetourTest, keep PredictionAccuracyIntegrationTest ignored with documented non-determinism), and documents an end-to-end fixture-refresh workflow and caveats.

Changes

Cohort / File(s) Summary
Docs & Guides
CLAUDE.md, README.md, docs/integration-tests.md, tools/wmata_capture/README.md
New integration-test refresh guide, non-determinism warning for PlaybackModule.runTrace, README header normalization, and updated capture README referencing the new workflow.
GTFS Helper Tools
tools/wmata_capture/subset_gtfs.py, tools/wmata_capture/find_detour_candidates.py
Added CLI scripts: subset_gtfs.py (route-based GTFS subsetting, CSV I/O helpers, main entry) and find_detour_candidates.py (rank AVL traces by off-route/detour metrics).
Integration Tests
transitclockIntegration/src/test/java/.../RecoverFromDetourTest.java, transitclockIntegration/src/test/java/.../prediction/PredictionAccuracyIntegrationTest.java
Removed @Ignore from RecoverFromDetourTest and updated fixture constants; PredictionAccuracyIntegrationTest still @Ignore with explicit non-determinism rationale and refreshed fixture paths.
Removed old fixtures (3T, S2)
transitclockIntegration/src/test/resources/gtfs/3T/*, transitclockIntegration/src/test/resources/gtfs/S2/*
Deleted legacy 3T and S2 GTFS fixture files (agency, calendar, routes, stops, trips, t.txt).
Added A40 fixtures
transitclockIntegration/src/test/resources/gtfs/A40/*
New complete A40 GTFS dataset: agency.txt, calendar.txt, calendar_dates.txt, feed_info.txt, routes.txt, stops.txt, trips.txt (large).
Added D20 fixtures
transitclockIntegration/src/test/resources/gtfs/D20/*
New complete D20 GTFS dataset and supporting files (agency, calendar, calendar_dates, feed_info, routes, stops, trips).
Added D40 fixtures
transitclockIntegration/src/test/resources/gtfs/D40/*
New complete D40 GTFS dataset and supporting files (agency, calendar, calendar_dates, feed_info, routes, stops, trips).

Sequence Diagram(s)

sequenceDiagram
  participant Dev as Developer
  participant Capture as wmata_capture tools
  participant Subset as subset_gtfs.py
  participant Finder as find_detour_candidates.py
  participant Repo as Repo test resources
  participant Tests as Integration tests

  Dev->>Capture: run capture.py to pull GTFS/AVL
  Capture->>Repo: write raw captures
  Dev->>Subset: run subset_gtfs.py (--route)
  Subset->>Repo: write per-route GTFS subset into test resources
  Dev->>Finder: run find_detour_candidates.py (--avl-dir)
  Finder->>Dev: return ranked detour candidate traces
  Dev->>Repo: promote selected AVL CSVs into fixtures
  Dev->>Tests: run integration tests against refreshed fixtures
  Tests->>Repo: read GTFS/AVL fixtures
  Tests-->>Dev: pass/fail (note: PredictionAccuracyIntegrationTest ignored due to non-determinism)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related issues

Possibly related PRs

  • Integration tests #9: Shares the same integration-test fixture refresh scope and wmata_capture tool additions; likely closely related.
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Fixup integration tests' is vague and generic, using non-descriptive language that doesn't convey specific information about the substantial changes made in this PR. Replace with a more descriptive title that highlights the main objective, such as 'Refresh WMATA integration test fixtures with D40, A40, and D20 routes' or 'Document integration test fixture refresh workflow and update fixture data'.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 fixup-integration-tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🧹 Nitpick comments (2)
transitclockIntegration/src/test/java/org/transitclock/integration_tests/RecoverFromDetourTest.java (1)

27-29: Assert vehicle presence before dereferencing it.

If playback/fixture drift causes lookup to miss, this currently fails as a generic NPE instead of a clear test failure reason.

Suggested fix
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
@@
 		PlaybackModule.runTrace(GTFS, AVL);
 		IpcVehicleComplete v = VehicleDataCache.getInstance().getVehicle(VEHICLE);
+		assertNotNull("Expected vehicle to be present after playback", v);
 		assertFalse(v.isLayover());
 		int adh = Math.abs(v.getRealTimeSchedAdh().getTemporalDifference());
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@transitclockIntegration/src/test/java/org/transitclock/integration_tests/RecoverFromDetourTest.java`
around lines 27 - 29, The test dereferences the result of
VehicleDataCache.getInstance().getVehicle(VEHICLE) (stored in IpcVehicleComplete
v) without checking for null, causing a NPE on v.isLayover() when the fixture
misses; add an explicit assertion that v is not null (e.g., assertNotNull with a
clear message referencing VEHICLE) immediately after retrieving v, then proceed
with assertFalse(v.isLayover()) and the adh calculation using
v.getRealTimeSchedAdh().getTemporalDifference().
transitclockIntegration/src/test/resources/gtfs/D40/trips.txt (1)

1-1610: Add automated GTFS fixture integrity checks for this large trips dataset.

Given the size of Line 1–Line 1610, please add a lightweight validation step to catch orphaned service_id, route_id, and optional shape_id references before tests run.

#!/bin/bash
set -euo pipefail

python - <<'PY'
import csv
from pathlib import Path

root = Path("transitclockIntegration/src/test/resources/gtfs")

def read_csv(path):
    with path.open(newline='', encoding='utf-8') as f:
        return list(csv.DictReader(f))

def validate_dataset(ds):
    p = root / ds
    trips_p = p / "trips.txt"
    cal_p = p / "calendar.txt"
    routes_p = p / "routes.txt"
    shapes_p = p / "shapes.txt"

    trips = read_csv(trips_p)
    calendar = read_csv(cal_p)
    routes = read_csv(routes_p)

    cal_ids = {r["service_id"] for r in calendar}
    route_ids = {r["route_id"] for r in routes}
    shape_ids = set()
    if shapes_p.exists():
        shape_ids = {r["shape_id"] for r in read_csv(shapes_p)}

    missing_service = sorted({t["service_id"] for t in trips if t["service_id"] not in cal_ids})
    missing_route = sorted({t["route_id"] for t in trips if t["route_id"] not in route_ids})
    missing_shape = sorted({t["shape_id"] for t in trips if shape_ids and t["shape_id"] not in shape_ids})

    print(f"[{ds}] trips={len(trips)}")
    print(f"[{ds}] missing service_ids: {missing_service[:10]}{' ...' if len(missing_service)>10 else ''}")
    print(f"[{ds}] missing route_ids: {missing_route[:10]}{' ...' if len(missing_route)>10 else ''}")
    if shapes_p.exists():
        print(f"[{ds}] missing shape_ids: {missing_shape[:10]}{' ...' if len(missing_shape)>10 else ''}")

    # non-zero exit if core referential integrity fails
    if missing_service or missing_route or (shapes_p.exists() and missing_shape):
        raise SystemExit(1)

for ds in ("D40",):
    validate_dataset(ds)
PY

Also run the integration suite with the intended profile to validate fixture behavior end-to-end:

mvn install -P include-integration-tests

Based on learnings: Run integration tests with mvn install -P include-integration-tests; they live in the transitclockIntegration module and are excluded by default.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@transitclockIntegration/src/test/resources/gtfs/D40/trips.txt` around lines 1
- 1610, Add a lightweight GTFS referential-integrity check that runs before
integration tests: implement the provided validate_dataset flow (use functions
read_csv and validate_dataset, root pointing at
transitclockIntegration/src/test/resources/gtfs) to load trips.txt,
calendar.txt, routes.txt and optional shapes.txt and fail fast if any trip
references a missing service_id, route_id, or shape_id; wire this script into
the test setup (pre-test step for the transitclockIntegration suite so it runs
before mvn install -P include-integration-tests) and ensure the script exits
non-zero when missing_service, missing_route, or missing_shape are non-empty.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/integration-tests.md`:
- Around line 78-87: The document contains conflicting guidance about the status
of PredictionAccuracyIntegrationTest (and similarly referenced at the block
around lines 226-231): one section lists it as `@Ignore` due to nondeterministic
baseline, while another instructs removing `@Ignore`; reconcile by updating the
runbook so a single canonical status and action are stated for
PredictionAccuracyIntegrationTest (and RecoverFromDetourTest if applicable):
explicitly mark it as “blocked by nondeterminism” or “ready to un-ignore”,
remove the contradictory instruction, and add a short rationale and next step
(e.g., reproduce/fix nondeterminism or a link to issue `#8`) so readers have
unambiguous state/action guidance referencing the test name
PredictionAccuracyIntegrationTest and the related issue `#8`.
- Around line 254-256: The fenced code block containing the CSV header line
"id,affectedByWaitStop,avlTime,configRev,creationTime,gtfsStopSeq,isArrival,predictionTime,routeId,schedBasedPred,stopId,tripId,vehicleId"
is missing a language identifier and triggers markdownlint MD040; update the
opening fence (the triple backticks immediately before that header) to include a
language such as csv or text (e.g., change ``` to ```csv) so the block is
properly identified.
- Line 137: The runbook contains a machine-specific absolute path ("cd
/Users/aaron/repos/onebusaway/transitime/tools/wmata_capture") which is not
portable; replace this hardcoded path with a relative repo path or a
parameterized/env-var placeholder (e.g., use "./tools/wmata_capture" or
"${REPO_ROOT}/tools/wmata_capture") and update the surrounding instructions to
document how to set REPO_ROOT or run from the repo root so contributors and CI
can run the command without editing the file.
- Around line 45-47: The docs currently show the wrong integration-test
invocation ("mvn -P include-integration-tests test"); update the text to use the
project's documented command by replacing that invocation with "mvn install -P
include-integration-tests" and add a brief note that these tests reside in the
transitclockIntegration module and are excluded by default so readers know why
the profile is required.

In `@tools/wmata_capture/find_detour_candidates.py`:
- Around line 129-140: Filter the accumulated results to actual detour
candidates before sorting/printing by keeping only entries where
longest_bracketed_off_run > 0 and ends_on_route is True (this is the candidate
definition used by score_vehicle results); do this right after the loop that
builds results (the loop iterating over avl_files that calls score_vehicle) and
before the results.sort and the print loop, then sort/print only the filtered
list (use args.top as before).
- Line 96: The median calculation for "median_dist_m" in
find_detour_candidates.py currently uses sorted(distances)[len(distances)//2],
which biases toward the upper middle for even-length lists; update the
assignment to compute a true median by either calling
statistics.median(distances) or by sorting distances and, if len(distances) is
even, averaging the two middle values (sorted_dist[n//2 - 1] and
sorted_dist[n//2]), otherwise taking the middle element—ensure you reference the
same "distances" sequence and replace the existing "median_dist_m" expression
accordingly.

In `@tools/wmata_capture/subset_gtfs.py`:
- Around line 73-81: After computing the GTFS subsets (kept_trips,
kept_trip_ids, kept_service_ids, kept_shape_ids and kept_stop_times), add a
fast-fail check that if kept_trips is empty or kept_stop_times is empty you
abort with a clear error (raise an exception or call sys.exit(1)) including the
route_id and which collection is empty; update the code around the kept_trips /
kept_stop_times computation (the variables kept_trips, kept_trip_ids,
kept_stop_times) to perform this validation before proceeding to write out files
so no empty slice is written.

---

Nitpick comments:
In
`@transitclockIntegration/src/test/java/org/transitclock/integration_tests/RecoverFromDetourTest.java`:
- Around line 27-29: The test dereferences the result of
VehicleDataCache.getInstance().getVehicle(VEHICLE) (stored in IpcVehicleComplete
v) without checking for null, causing a NPE on v.isLayover() when the fixture
misses; add an explicit assertion that v is not null (e.g., assertNotNull with a
clear message referencing VEHICLE) immediately after retrieving v, then proceed
with assertFalse(v.isLayover()) and the adh calculation using
v.getRealTimeSchedAdh().getTemporalDifference().

In `@transitclockIntegration/src/test/resources/gtfs/D40/trips.txt`:
- Around line 1-1610: Add a lightweight GTFS referential-integrity check that
runs before integration tests: implement the provided validate_dataset flow (use
functions read_csv and validate_dataset, root pointing at
transitclockIntegration/src/test/resources/gtfs) to load trips.txt,
calendar.txt, routes.txt and optional shapes.txt and fail fast if any trip
references a missing service_id, route_id, or shape_id; wire this script into
the test setup (pre-test step for the transitclockIntegration suite so it runs
before mvn install -P include-integration-tests) and ensure the script exits
non-zero when missing_service, missing_route, or missing_shape are non-empty.
🪄 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: f0b0aa65-cceb-4aac-94e9-3a59d709002f

📥 Commits

Reviewing files that changed from the base of the PR and between b579ed7 and 5f5063e.

⛔ Files ignored due to path filters (6)
  • transitclockIntegration/src/test/resources/avl/3T_3757.csv is excluded by !**/*.csv
  • transitclockIntegration/src/test/resources/avl/A40_3151.csv is excluded by !**/*.csv
  • transitclockIntegration/src/test/resources/avl/D20_7198.csv is excluded by !**/*.csv
  • transitclockIntegration/src/test/resources/avl/D40_5506.csv is excluded by !**/*.csv
  • transitclockIntegration/src/test/resources/avl/S2_2113.csv is excluded by !**/*.csv
  • transitclockIntegration/src/test/resources/pred/S2_2113.csv is excluded by !**/*.csv
📒 Files selected for processing (52)
  • CLAUDE.md
  • README.md
  • docs/integration-tests.md
  • tools/wmata_capture/README.md
  • tools/wmata_capture/find_detour_candidates.py
  • tools/wmata_capture/subset_gtfs.py
  • transitclockIntegration/src/test/java/org/transitclock/integration_tests/RecoverFromDetourTest.java
  • transitclockIntegration/src/test/java/org/transitclock/integration_tests/prediction/PredictionAccuracyIntegrationTest.java
  • transitclockIntegration/src/test/resources/gtfs/3T/agency.txt
  • transitclockIntegration/src/test/resources/gtfs/3T/calendar.txt
  • transitclockIntegration/src/test/resources/gtfs/3T/calendar_dates.txt
  • transitclockIntegration/src/test/resources/gtfs/3T/routes.txt
  • transitclockIntegration/src/test/resources/gtfs/3T/shapes.txt
  • transitclockIntegration/src/test/resources/gtfs/3T/stop_times.txt
  • transitclockIntegration/src/test/resources/gtfs/3T/stops.txt
  • transitclockIntegration/src/test/resources/gtfs/3T/trips.txt
  • transitclockIntegration/src/test/resources/gtfs/A40/agency.txt
  • transitclockIntegration/src/test/resources/gtfs/A40/calendar.txt
  • transitclockIntegration/src/test/resources/gtfs/A40/calendar_dates.txt
  • transitclockIntegration/src/test/resources/gtfs/A40/feed_info.txt
  • transitclockIntegration/src/test/resources/gtfs/A40/routes.txt
  • transitclockIntegration/src/test/resources/gtfs/A40/shapes.txt
  • transitclockIntegration/src/test/resources/gtfs/A40/stop_times.txt
  • transitclockIntegration/src/test/resources/gtfs/A40/stops.txt
  • transitclockIntegration/src/test/resources/gtfs/A40/trips.txt
  • transitclockIntegration/src/test/resources/gtfs/D20/agency.txt
  • transitclockIntegration/src/test/resources/gtfs/D20/calendar.txt
  • transitclockIntegration/src/test/resources/gtfs/D20/calendar_dates.txt
  • transitclockIntegration/src/test/resources/gtfs/D20/feed_info.txt
  • transitclockIntegration/src/test/resources/gtfs/D20/routes.txt
  • transitclockIntegration/src/test/resources/gtfs/D20/shapes.txt
  • transitclockIntegration/src/test/resources/gtfs/D20/stop_times.txt
  • transitclockIntegration/src/test/resources/gtfs/D20/stops.txt
  • transitclockIntegration/src/test/resources/gtfs/D20/trips.txt
  • transitclockIntegration/src/test/resources/gtfs/D40/agency.txt
  • transitclockIntegration/src/test/resources/gtfs/D40/calendar.txt
  • transitclockIntegration/src/test/resources/gtfs/D40/calendar_dates.txt
  • transitclockIntegration/src/test/resources/gtfs/D40/feed_info.txt
  • transitclockIntegration/src/test/resources/gtfs/D40/routes.txt
  • transitclockIntegration/src/test/resources/gtfs/D40/shapes.txt
  • transitclockIntegration/src/test/resources/gtfs/D40/stop_times.txt
  • transitclockIntegration/src/test/resources/gtfs/D40/stops.txt
  • transitclockIntegration/src/test/resources/gtfs/D40/trips.txt
  • transitclockIntegration/src/test/resources/gtfs/S2/agency.txt
  • transitclockIntegration/src/test/resources/gtfs/S2/calendar.txt
  • transitclockIntegration/src/test/resources/gtfs/S2/calendar_dates.txt
  • transitclockIntegration/src/test/resources/gtfs/S2/routes.txt
  • transitclockIntegration/src/test/resources/gtfs/S2/shapes.txt
  • transitclockIntegration/src/test/resources/gtfs/S2/stop_times.txt
  • transitclockIntegration/src/test/resources/gtfs/S2/stops.txt
  • transitclockIntegration/src/test/resources/gtfs/S2/t.txt
  • transitclockIntegration/src/test/resources/gtfs/S2/trips.txt
💤 Files with no reviewable changes (11)
  • transitclockIntegration/src/test/resources/gtfs/S2/t.txt
  • transitclockIntegration/src/test/resources/gtfs/S2/agency.txt
  • transitclockIntegration/src/test/resources/gtfs/3T/agency.txt
  • transitclockIntegration/src/test/resources/gtfs/S2/routes.txt
  • transitclockIntegration/src/test/resources/gtfs/3T/calendar.txt
  • transitclockIntegration/src/test/resources/gtfs/S2/calendar.txt
  • transitclockIntegration/src/test/resources/gtfs/3T/stops.txt
  • transitclockIntegration/src/test/resources/gtfs/3T/routes.txt
  • transitclockIntegration/src/test/resources/gtfs/S2/stops.txt
  • transitclockIntegration/src/test/resources/gtfs/3T/trips.txt
  • transitclockIntegration/src/test/resources/gtfs/S2/trips.txt

Comment thread docs/integration-tests.md
Comment on lines +45 to +47
- **Step 5 validate — DONE (for non-ignored tests).** `mvn -P
include-integration-tests test` on the module passes: 3 tests green
(detour + two 5A), 1 skipped (testPredictions). Matches the pre-

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Use the documented integration-test invocation.

Line 45-47 should use the project’s integration-test command to avoid confusion in future reruns.

Based on learnings: Run integration tests with mvn install -P include-integration-tests; they live in the transitclockIntegration module and are excluded by default.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/integration-tests.md` around lines 45 - 47, The docs currently show the
wrong integration-test invocation ("mvn -P include-integration-tests test");
update the text to use the project's documented command by replacing that
invocation with "mvn install -P include-integration-tests" and add a brief note
that these tests reside in the transitclockIntegration module and are excluded
by default so readers know why the profile is required.

Comment thread docs/integration-tests.md
Comment thread docs/integration-tests.md Outdated
Comment thread docs/integration-tests.md Outdated
"vehicle": avl_path.stem,
"rows": len(rows),
"max_dist_m": max(distances),
"median_dist_m": sorted(distances)[len(distances) // 2],

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

median_dist_m is not a true median for even-length traces.

Selecting sorted(distances)[len//2] biases to the upper middle value when row count is even.

Suggested fix
 import argparse
 import csv
 import math
+import statistics
 import sys
 from pathlib import Path
@@
-        "median_dist_m": sorted(distances)[len(distances) // 2],
+        "median_dist_m": statistics.median(distances),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tools/wmata_capture/find_detour_candidates.py` at line 96, The median
calculation for "median_dist_m" in find_detour_candidates.py currently uses
sorted(distances)[len(distances)//2], which biases toward the upper middle for
even-length lists; update the assignment to compute a true median by either
calling statistics.median(distances) or by sorting distances and, if
len(distances) is even, averaging the two middle values (sorted_dist[n//2 - 1]
and sorted_dist[n//2]), otherwise taking the middle element—ensure you reference
the same "distances" sequence and replace the existing "median_dist_m"
expression accordingly.

Comment thread tools/wmata_capture/find_detour_candidates.py Outdated
Comment on lines +73 to +81
trips_fields, trips_rows = read_csv(input_dir / "trips.txt")
kept_trips = [t for t in trips_rows if t["route_id"] == route_id]
kept_trip_ids = {t["trip_id"] for t in kept_trips}
kept_service_ids = {t["service_id"] for t in kept_trips}
kept_shape_ids = {t["shape_id"] for t in kept_trips if t.get("shape_id")}

st_fields, st_rows = read_csv(input_dir / "stop_times.txt")
kept_stop_times = [r for r in st_rows if r["trip_id"] in kept_trip_ids]
kept_stop_ids = {r["stop_id"] for r in kept_stop_times}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fail fast when subsetting yields no usable trips/stop_times.

Right now an empty route slice still writes files, which can hide bad input/capture windows until later tests fail.

Suggested safeguard
     trips_fields, trips_rows = read_csv(input_dir / "trips.txt")
     kept_trips = [t for t in trips_rows if t["route_id"] == route_id]
+    if not kept_trips:
+        raise SystemExit(
+            f"No trips found for route_short_name={route_short_name!r} (route_id={route_id!r})"
+        )
     kept_trip_ids = {t["trip_id"] for t in kept_trips}
     kept_service_ids = {t["service_id"] for t in kept_trips}
     kept_shape_ids = {t["shape_id"] for t in kept_trips if t.get("shape_id")}

     st_fields, st_rows = read_csv(input_dir / "stop_times.txt")
     kept_stop_times = [r for r in st_rows if r["trip_id"] in kept_trip_ids]
+    if not kept_stop_times:
+        raise SystemExit(
+            f"No stop_times found for route_short_name={route_short_name!r} (route_id={route_id!r})"
+        )
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tools/wmata_capture/subset_gtfs.py` around lines 73 - 81, After computing the
GTFS subsets (kept_trips, kept_trip_ids, kept_service_ids, kept_shape_ids and
kept_stop_times), add a fast-fail check that if kept_trips is empty or
kept_stop_times is empty you abort with a clear error (raise an exception or
call sys.exit(1)) including the route_id and which collection is empty; update
the code around the kept_trips / kept_stop_times computation (the variables
kept_trips, kept_trip_ids, kept_stop_times) to perform this validation before
proceeding to write out files so no empty slice is written.

- docs/integration-tests.md: update "Current state" table and fixture
  layout to reflect the completed refresh (A40/D40/D20, D40_5506 avl,
  RecoverFromDetourTest passing). Annotate Step 4 edits with what's
  already done, and gate the pred-baseline regeneration section behind
  the non-determinism blocker so readers don't un-ignore the test
  prematurely. Drop the hardcoded /Users/ path in Step 1.
- find_detour_candidates.py: filter results to actual detour candidates
  (bracketed off-route run that returns on-route) before sorting and
  printing, so the top-N list can't surface non-candidates.
@aaronbrethorst
aaronbrethorst merged commit 5970891 into develop Apr 24, 2026
2 of 3 checks passed
@aaronbrethorst
aaronbrethorst deleted the fixup-integration-tests branch April 24, 2026 17:42
0x3mr pushed a commit to 0x3mr/trip-updates that referenced this pull request Apr 30, 2026
jQuery UI was vendored in two locations (~250KB of JS+CSS+images) but
only two of its widgets were actually used:

- .datepicker() in two report-param JSPs — replaced with air-datepicker v2
  (already vendored at javascript/air-datepicker/, never wired up before).
  Same jQuery-plugin call shape ($("#x").datepicker({...})). Cross-field
  min/maxDate constraint is now wired via onSelect callbacks.

- .tooltip() — global init in transitime.js plus per-page wirings on
  Select2 containers in 8 form-param JSPs. Dropped entirely; native
  browser tooltips on the underlying <input>/<select> remain. The two
  "tooltip-disable" workarounds in maps/map.jsp and synoptic/index.jsp
  existed solely to suppress side effects of the global init and
  disappear with it.

Also:
- $('#mapTitle').hide('fade', 1000) — a jQuery UI effect — replaced
  with core jQuery $.fadeOut(1000).
- transitime.js now has a cache-buster query string, mirroring the
  existing pattern for general.css.
- <br>-based HTML in date/time tooltip title attrs converted to
  &OneBusAway#10; line-break entities so native browser tooltips render
  multi-line content correctly.

Deleted:
- transitclockWebapp/src/main/webapp/jquery-ui/ (full vendored library
  with 12 files including 10 PNG images, full CSS+JS, and a demo).
- transitclockWebapp/src/main/webapp/javascript/jquery-ui.{js,min.js}
  (duplicate vendored copy in the javascript/ dir).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant