admin/Sportstime
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Audit and fix 52 test correctness issues across 22 files

Browse Source 
Systematic audit of 1,191 tests found tests written to pass rather than
verify correctness. Key fixes:

Infrastructure:
- TestClock: fixed timezone from .current to America/New_York (deterministic)
- TestFixtures: added 1.3x road routing factor to match production
- ItineraryTestHelpers: real per-city coordinates instead of hardcoded (40,-80)

Planning tests:
- Added missing Scenario E factory dispatch tests
- Tightened 12 loose assertions (>= 1 → == 8.0, > 0 → range checks)
- Fixed 4 no-op tests that accepted both success and failure
- Fixed wrong repeat-city invariant (was checking same-day, not different-day)
- Fixed tautological assertion in missing-stadium edge case

Services/Domain/Export tests:
- Replaced 4 placeholder tests (#expect(true)) with real assertions
- Fixed tautological assertions in POISearchServiceTests
- Fixed Chicago coordinate in RegionMapSelectorTests (-89 → -87.6553)
- Added sort order verification to ItineraryRowFlatteningTests

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Trey T
2026-04-04 23:00:46 -05:00
parent 9b622f8bbb
commit a6f538dfed
23 changed files with 265 additions and 109 deletions
Download Patch File Download Diff File Expand all files Collapse all files

34
SportsTimeTests/Planning/GameDAGRouterTests.swift
View File

@@ -531,10 +531,18 @@ struct GameDAGRouterTests {
constraints: constraints
)
// May return routes with just game1, or empty
#expect(routes.allSatisfy { route in
route.allSatisfy { game in stadiums[game.stadiumId] != nil || game.id == game2.id }
})
// Multi-game routes should not include games with missing stadiums
// (the router can't build transitions without stadium coordinates).
// Single-game routes may still include them since no transition is needed.
for route in routes where route.count > 1 {
for game in route {
#expect(stadiums[game.stadiumId] != nil,
"Multi-game route should not include games with missing stadiums")
}
}
// The router should still return routes (at least the valid single-game route)
#expect(!routes.isEmpty, "Should return at least the valid game as a single-game route")
}
// MARK: - Route Preference Tests
@@ -576,7 +584,7 @@ struct GameDAGRouterTests {
let directMiles = totalMiles(for: directFirst, stadiums: stadiums)
let scenicMiles = totalMiles(for: scenicFirst, stadiums: stadiums)
// Direct should tend toward lower mileage routes being ranked first
#expect(directMiles <= scenicMiles + 500, "Direct route should not be significantly longer than scenic")
#expect(directMiles <= scenicMiles, "Direct first route (\(Int(directMiles))mi) should be <= scenic first route (\(Int(scenicMiles))mi)")
}
}
@@ -606,6 +614,16 @@ struct GameDAGRouterTests {
Set(route.compactMap { stadiums[$0.stadiumId]?.city }).count
}.max() ?? 0
#expect(maxCities >= 2, "Scenic should produce multi-city routes")
let directRoutes2 = GameDAGRouter.findRoutes(
games: games, stadiums: stadiums, constraints: constraints,
routePreference: .direct
)
if let sFirst = scenicRoutes.first, let dFirst = directRoutes2.first {
let sCities = Set(sFirst.compactMap { stadiums[$0.stadiumId]?.city }).count
let dCities = Set(dFirst.compactMap { stadiums[$0.stadiumId]?.city }).count
#expect(sCities >= dCities, "Scenic first route should have >= cities than direct first route")
}
}
@Test("routePreference: balanced matches default behavior")
@@ -629,6 +647,12 @@ struct GameDAGRouterTests {
// Both should produce the same routes (balanced is default)
#expect(balancedRoutes.count == defaultRoutes.count)
if let bFirst = balancedRoutes.first, let dFirst = defaultRoutes.first {
let bIds = bFirst.map { $0.id }
let dIds = dFirst.map { $0.id }
#expect(bIds == dIds, "Balanced and default should produce identical first route")
}
}
// MARK: - Route Preference Scoring Tests

48
SportsTimeTests/Planning/ImprovementPlanTDDTests.swift
View File

@@ -63,6 +63,25 @@ struct Phase1A_RoutePreferenceTests {
#expect(route[i].startTime <= route[i + 1].startTime)
}
}
// Direct should produce lower-mileage first routes than scenic
if let directFirst = directRoutes.first, let scenicFirst = scenicRoutes.first {
let directMiles = directFirst.compactMap { game -> Double? in
guard let idx = directFirst.firstIndex(where: { $0.id == game.id }),
idx > 0,
let from = stadiums[directFirst[idx - 1].stadiumId],
let to = stadiums[game.stadiumId] else { return nil }
return TravelEstimator.haversineDistanceMiles(from: from.coordinate, to: to.coordinate) * 1.3
}.reduce(0, +)
let scenicMiles = scenicFirst.compactMap { game -> Double? in
guard let idx = scenicFirst.firstIndex(where: { $0.id == game.id }),
idx > 0,
let from = stadiums[scenicFirst[idx - 1].stadiumId],
let to = stadiums[game.stadiumId] else { return nil }
return TravelEstimator.haversineDistanceMiles(from: from.coordinate, to: to.coordinate) * 1.3
}.reduce(0, +)
#expect(directMiles <= scenicMiles, "Direct first route should have <= mileage than scenic first route")
}
}
@Test("findRoutes accepts routePreference parameter for all values")
@@ -165,6 +184,9 @@ struct Phase1B_ScenarioERegionTests {
let nycTeam = TestFixtures.team(id: "team_nyc", name: "NYC Team", sport: .mlb, city: "New York")
let bosTeam = TestFixtures.team(id: "team_bos", name: "BOS Team", sport: .mlb, city: "Boston")
// With 2 teams, teamFirstMaxDays = 4. The sliding window needs the game
// span to be wide enough so a 4-day window can contain both games.
// Space games 3 days apart so a window from June 1 to June 5 covers both.
let game1 = TestFixtures.game(
id: "nyc_home", city: "New York",
dateTime: baseDate,
@@ -173,7 +195,7 @@ struct Phase1B_ScenarioERegionTests {
)
let game2 = TestFixtures.game(
id: "bos_home", city: "Boston",
dateTime: TestClock.calendar.date(byAdding: .day, value: 1, to: baseDate)!,
dateTime: TestClock.calendar.date(byAdding: .day, value: 3, to: baseDate)!,
homeTeamId: "team_bos",
stadiumId: "stadium_mlb_boston"
)
@@ -199,13 +221,11 @@ struct Phase1B_ScenarioERegionTests {
let result = planner.plan(request: request)
// Should succeed with both nearby East Coast teams.
// Failure is also OK if driving constraints prevent it.
switch result {
case .success(let options):
#expect(!options.isEmpty, "Should find routes for NYC + Boston")
case .failure:
break // Acceptable — driving constraints may prevent a valid route
guard case .success(let options) = result else {
Issue.record("Expected .success for NYC + Boston (nearby East Coast teams), got \(result)")
return
}
#expect(!options.isEmpty, "Should find routes for NYC + Boston")
}
}
@@ -588,15 +608,17 @@ struct Phase2D_ExclusionWarningTests {
// If all routes had repeat cities, failure is also acceptable
return
}
// Every returned route must have unique cities per calendar day
for option in options {
let calendar = Calendar.current
var cityDays: Set<String> = []
// Group stops by city, check no city appears on multiple calendar days
var cityDays: [String: Set<Date>] = [:]
let calendar = TestClock.calendar
for stop in option.stops {
let day = calendar.startOfDay(for: stop.arrivalDate)
let key = "\(stop.city.lowercased())_\(day.timeIntervalSince1970)"
#expect(!cityDays.contains(key), "Route should not visit \(stop.city) on the same day twice")
cityDays.insert(key)
let city = stop.city.lowercased()
cityDays[city, default: []].insert(day)
}
for (city, days) in cityDays {
#expect(days.count <= 1, "City '\(city)' appears on \(days.count) different days — repeat city violation")
}
}
}

4
SportsTimeTests/Planning/ItineraryBuilderTests.swift
View File

@@ -60,8 +60,8 @@ struct ItineraryBuilderTests {
#expect(result != nil)
#expect(result?.stops.count == 2)
#expect(result?.travelSegments.count == 1)
#expect(result?.totalDrivingHours ?? 0 > 0)
#expect(result?.totalDistanceMiles ?? 0 > 0)
#expect((3.0...6.0).contains(result?.totalDrivingHours ?? 0), "NYC→Boston should be 3-6 hours driving")
#expect((200...400).contains(result?.totalDistanceMiles ?? 0), "NYC→Boston should be 200-400 road miles")
}
@Test("build: three stops creates two segments")

8
SportsTimeTests/Planning/PlanningModelsTests.swift
View File

@@ -341,8 +341,11 @@ struct PlanningModelsTests {
#expect(!stop.hasGames)
}
@Test("equality based on id only")
func equality_basedOnId() {
@Test("self-equality: same instance is equal to itself")
func self_equality() {
// ItineraryStop uses auto-generated UUID ids, so two separately constructed
// instances will always have different ids. Self-equality is the only
// meaningful equality test for this type.
let stop1 = ItineraryStop(
city: "New York",
state: "NY",
@@ -354,7 +357,6 @@ struct PlanningModelsTests {
firstGameStart: nil
)
// Same id via same instance
#expect(stop1 == stop1)
}

17
SportsTimeTests/Planning/PlanningPipelineBugRegressionTests.swift
View File

@@ -703,12 +703,23 @@ struct Bug15_DateArithmeticTests {
let planner = ScenarioBPlanner()
let result = planner.plan(request: request)
// Should not crash — verify we get a valid result (success or failure, not a crash)
// Bug #15 was a force-unwrap crash in date arithmetic. The fix ensures safe
// optional unwrapping. Both outcomes are acceptable here because:
// - .success: date arithmetic worked and a route was found
// - .failure(.noValidRoutes): date arithmetic worked but driving constraints
// or game spacing prevented a valid route (Boston→NYC in the available window)
// The critical assertion is that this code path does NOT crash (no force-unwrap trap).
switch result {
case .success(let options):
#expect(!options.isEmpty, "If success, should have at least one option")
case .failure:
break // Failure is acceptable — the point is it didn't crash
case .failure(let failure):
// Any planning-related failure is acceptable — the critical assertion is
// that the code path does NOT crash (no force-unwrap trap).
let acceptableReasons: [PlanningFailure.FailureReason] = [
.noValidRoutes, .noGamesInRange, .constraintsUnsatisfiable, .missingDateRange
]
#expect(acceptableReasons.contains { $0 == failure.reason },
"Expected a planning-related failure, got \(failure.reason)")
}
}
}

2
SportsTimeTests/Planning/RouteFiltersTests.swift
View File

@@ -55,6 +55,8 @@ struct RouteFiltersTests {
let result = RouteFilters.filterRepeatCities([violating, valid], allow: false)
#expect(result.count == 1)
let survivingCities = Set(result[0].stops.map { $0.city })
#expect(survivingCities.contains("Boston"), "The surviving option should be the non-violating route containing Boston")
}
@Test("filterRepeatCities: empty options returns empty")

11
SportsTimeTests/Planning/ScenarioEPlannerTests.swift
View File

@@ -685,15 +685,16 @@ struct ScenarioEPlannerTests {
let result = planner.plan(request: request)
// Should fail because driving constraint cannot be met
// Should fail because driving constraint cannot be met (NYC→LA is ~40h, single driver max 8h/day)
guard case .failure(let failure) = result else {
Issue.record("Expected failure when driving constraint cannot be met")
Issue.record("Expected failure when driving constraint cannot be met (NYC→LA with 1 driver, 8h max)")
return
}
// Could be noValidRoutes or constraintsUnsatisfiable
let validFailures: [PlanningFailure.FailureReason] = [.noValidRoutes, .constraintsUnsatisfiable]
#expect(validFailures.contains { $0 == failure.reason }, "Should fail due to route constraints")
// Verify the failure is specifically about driving/route constraints, not a generic error
let validFailures: [PlanningFailure.FailureReason] = [.noValidRoutes, .constraintsUnsatisfiable, .drivingExceedsLimit]
#expect(validFailures.contains { $0 == failure.reason },
"Expected driving-related failure (.noValidRoutes, .constraintsUnsatisfiable, or .drivingExceedsLimit), got \(failure.reason)")
}
// MARK: - E4. Edge Case Tests

38
SportsTimeTests/Planning/ScenarioPlannerFactoryTests.swift
View File

@@ -149,6 +149,44 @@ struct ScenarioPlannerFactoryTests {
#expect(planner is ScenarioBPlanner, "B should take priority over C")
}
@Test("planner: teamFirst with 2+ teams returns ScenarioEPlanner")
func planner_teamFirst_returnsScenarioE() {
let prefs = TripPreferences(
planningMode: .teamFirst,
sports: [.mlb],
startDate: TestClock.now,
endDate: TestClock.now.addingTimeInterval(86400 * 7),
leisureLevel: .moderate,
lodgingType: .hotel,
numberOfDrivers: 1,
selectedTeamIds: ["team-1", "team-2"]
)
let request = makeRequest(preferences: prefs)
let planner = ScenarioPlannerFactory.planner(for: request)
#expect(planner is ScenarioEPlanner)
}
@Test("classify: teamFirst with 2+ teams returns scenarioE")
func classify_teamFirst_returnsScenarioE() {
let prefs = TripPreferences(
planningMode: .teamFirst,
sports: [.mlb],
startDate: TestClock.now,
endDate: TestClock.now.addingTimeInterval(86400 * 7),
leisureLevel: .moderate,
lodgingType: .hotel,
numberOfDrivers: 1,
selectedTeamIds: ["team-1", "team-2"]
)
let request = makeRequest(preferences: prefs)
let scenario = ScenarioPlannerFactory.classify(request)
#expect(scenario == .scenarioE)
}
// MARK: - Specification Tests: classify()
@Test("classify: followTeamId returns scenarioD")

16
SportsTimeTests/Planning/TravelEstimatorTests.swift
View File

@@ -232,18 +232,16 @@ struct TravelEstimatorTests {
@Test("calculateTravelDays: all dates are start of day")
func calculateTravelDays_allDatesAreStartOfDay() {
let calendar = TestClock.calendar
// Use a specific time that's not midnight
var components = calendar.dateComponents([.year, .month, .day], from: TestClock.now)
components.hour = 14
components.minute = 30
let departure = calendar.date(from: components)!
// Production calculateTravelDays uses Calendar.current for startOfDay,
// so assert with Calendar.current to match.
let departure = TestFixtures.date(year: 2026, month: 6, day: 15, hour: 14, minute: 30)
let days = TravelEstimator.calculateTravelDays(departure: departure, drivingHours: 20)
let systemCalendar = Calendar.current
for day in days {
let hour = calendar.component(.hour, from: day)
let minute = calendar.component(.minute, from: day)
let hour = systemCalendar.component(.hour, from: day)
let minute = systemCalendar.component(.minute, from: day)
#expect(hour == 0 && minute == 0, "Expected midnight, got \(hour):\(minute)")
}
}
@@ -411,7 +409,7 @@ struct TravelEstimatorTests {
func edge_negativeDrivingHours() {
let departure = TestClock.now
let days = TravelEstimator.calculateTravelDays(departure: departure, drivingHours: -5)
#expect(days.count >= 1, "Negative hours should still return at least 1 day")
#expect(days.count == 1, "Negative hours should be treated as zero driving, returning exactly 1 day")
}
// MARK: - Helper Methods

2
SportsTimeTests/Planning/TravelSegmentIntegrityTests.swift
View File

@@ -533,7 +533,7 @@ struct TravelIntegrity_EdgeCaseTests {
let result = ItineraryBuilder.build(stops: stops, constraints: .default)
#expect(result != nil, "Same-city stops should build")
#expect(result!.travelSegments.count == 1, "Must still have segment")
// Distance should be very small (same coords)
#expect(result!.travelSegments[0].estimatedDistanceMiles < 1, "Same-city distance should be near zero")
}
@Test("Cross-country trip (NYC→LA) rejected with 1 driver, feasible with 2")

10
SportsTimeTests/Planning/TripPlanningEngineTests.swift
View File

@@ -32,7 +32,7 @@ struct TripPlanningEngineTests {
func drivingConstraints_clampsNegativeDrivers() {
let constraints = DrivingConstraints(numberOfDrivers: -5, maxHoursPerDriverPerDay: 8.0)
#expect(constraints.numberOfDrivers == 1)
#expect(constraints.maxDailyDrivingHours >= 1.0)
#expect(constraints.maxDailyDrivingHours == 8.0)
}
@Test("DrivingConstraints: clamps zero hours to minimum")
@@ -89,9 +89,11 @@ struct TripPlanningEngineTests {
func invariant_totalDriverHoursPositive() {
let prefs1 = TripPreferences(numberOfDrivers: 1)
#expect(prefs1.totalDriverHoursPerDay > 0)
#expect(prefs1.totalDriverHoursPerDay == 8.0) // 1 driver × 8 hrs
let prefs2 = TripPreferences(numberOfDrivers: 3, maxDrivingHoursPerDriver: 4)
#expect(prefs2.totalDriverHoursPerDay > 0)
#expect(prefs2.totalDriverHoursPerDay == 12.0) // 3 drivers × 4 hrs
}
@Test("Invariant: effectiveTripDuration >= 1")
@@ -102,6 +104,10 @@ struct TripPlanningEngineTests {
let prefs = TripPreferences(tripDuration: duration)
#expect(prefs.effectiveTripDuration >= 1)
}
// Verify specific value for nil duration with default dates
let prefsNil = TripPreferences(tripDuration: nil)
#expect(prefsNil.effectiveTripDuration == 8) // Default 7-day range = 8 days inclusive
}
// MARK: - Travel Segment Validation
@@ -189,7 +195,7 @@ struct TripPlanningEngineTests {
}
}
@Test("planTrip: invalid options are filtered out")
@Test("ItineraryOption.isValid: correctly validates segment count")
func planTrip_invalidOptions_areFilteredOut() {
// Create a valid ItineraryOption manually with wrong segment count
let stop1 = ItineraryStop(