diff --git a/cmd/api/app.go b/cmd/api/app.go index e9487297..6e04c2c7 100644 --- a/cmd/api/app.go +++ b/cmd/api/app.go @@ -253,7 +253,10 @@ func Run(ctx context.Context, srv *http.Server, coreApp *app.Application, api *r // Then shutdown GTFS manager (stops data fetching - the lowest-level dependency) if coreApp.GtfsManager != nil { - coreApp.GtfsManager.Shutdown() + err := coreApp.GtfsManager.Shutdown(shutdownCtx) + if err != nil { + logger.Error("Error occurred while shutting down GTFS manager", "error", err) + } } logger.Info("server exited") diff --git a/internal/gtfs/advanced_direction_calculator_test.go b/internal/gtfs/advanced_direction_calculator_test.go index ad0ce289..c6f8a616 100644 --- a/internal/gtfs/advanced_direction_calculator_test.go +++ b/internal/gtfs/advanced_direction_calculator_test.go @@ -544,8 +544,12 @@ func TestMain(m *testing.M) { // Global Teardown // If sharedManager was initialized during tests, shut it down now. + ctx := context.Background() if sharedManager != nil { - sharedManager.Shutdown() + err := sharedManager.Shutdown(ctx) + if err != nil { + _, _ = os.Stderr.WriteString("Error occurred while shutting down shared GTFS manager: " + err.Error() + "\n") + } } // Exit with the test result code diff --git a/internal/gtfs/gtfs_manager.go b/internal/gtfs/gtfs_manager.go index d475f6d8..3c3bdc93 100644 --- a/internal/gtfs/gtfs_manager.go +++ b/internal/gtfs/gtfs_manager.go @@ -65,6 +65,7 @@ type Manager struct { shutdownChan chan struct{} wg sync.WaitGroup shutdownOnce sync.Once + closeDBOnce sync.Once stopSpatialIndex *rtree.RTree blockLayoverIndices map[string][]*BlockLayoverIndex regionBounds *RegionBounds @@ -379,10 +380,13 @@ func (manager *Manager) SetGtfsURL(url string) { } // Shutdown gracefully shuts down the manager and its background goroutines -func (manager *Manager) Shutdown() { +func (manager *Manager) Shutdown(ctx context.Context) error { manager.shutdownOnce.Do(func() { close(manager.shutdownChan) - manager.wg.Wait() + }) + + // Always close DB exactly once, regardless of path + defer manager.closeDBOnce.Do(func() { if manager.GtfsDB != nil { if err := manager.GtfsDB.Close(); err != nil { logger := slog.Default().With(slog.String("component", "gtfs_manager")) @@ -390,6 +394,19 @@ func (manager *Manager) Shutdown() { } } }) + + done := make(chan struct{}) + go func() { + manager.wg.Wait() + close(done) + }() + + select { + case <-done: + return nil + case <-ctx.Done(): + return fmt.Errorf("shutdown timeout exceeded: %w", ctx.Err()) + } } // RLock acquires the static data read lock. diff --git a/internal/gtfs/gtfs_manager_test.go b/internal/gtfs/gtfs_manager_test.go index b391207d..eda0a2e1 100644 --- a/internal/gtfs/gtfs_manager_test.go +++ b/internal/gtfs/gtfs_manager_test.go @@ -305,7 +305,12 @@ func TestManager_GetVehicleForTrip(t *testing.T) { // We use isolated GTFSManager here instead of shared test components because we want to control the real-time vehicles for this test. manager, err := InitGTFSManager(ctx, gtfsConfig) assert.Nil(t, err) - defer manager.Shutdown() + + defer func() { + if err := manager.Shutdown(context.Background()); err != nil { + t.Errorf("Error occurred while shutting down GTFS manager: %v", err) + } + }() trip := >fs.Trip{ ID: gtfs.TripID{ID: "5735633"}, @@ -407,7 +412,12 @@ func TestRoutesForAgencyID_MapOptimization(t *testing.T) { } manager, err := InitGTFSManager(ctx, gtfsConfig) require.NoError(t, err, "Failed to initialize manager") - defer manager.Shutdown() + defer func() { + err := manager.Shutdown(ctx) + if err != nil { + t.Errorf("Error occurred while shutting down GTFS manager: %v", err) + } + }() targetAgencyID := "25" expectedRouteCount := 13 @@ -435,18 +445,24 @@ func TestRoutesForAgencyID_MapOptimization(t *testing.T) { } func TestRoutesForAgencyID_ConcurrentAccess(t *testing.T) { - ctx := context.Background() + setupCtx := context.Background() gtfsConfig := Config{ GtfsURL: models.GetFixturePath(t, "raba.zip"), GTFSDataPath: ":memory:", Env: appconf.Test, } - manager, err := InitGTFSManager(ctx, gtfsConfig) + manager, err := InitGTFSManager(setupCtx, gtfsConfig) require.NoError(t, err) - defer manager.Shutdown() - ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer func() { + err := manager.Shutdown(context.Background()) + if err != nil { + t.Errorf("Error occurred while shutting down GTFS manager: %v", err) + } + }() + + runCtx, cancel := context.WithTimeout(context.Background(), 2*time.Second) defer cancel() var wg sync.WaitGroup @@ -459,7 +475,7 @@ func TestRoutesForAgencyID_ConcurrentAccess(t *testing.T) { defer wg.Done() for { select { - case <-ctx.Done(): + case <-runCtx.Done(): return default: manager.RLock() @@ -488,7 +504,7 @@ func TestRoutesForAgencyID_ConcurrentAccess(t *testing.T) { for { select { - case <-ctx.Done(): + case <-runCtx.Done(): return default: manager.setStaticGTFS(staticData) @@ -517,7 +533,11 @@ func BenchmarkRoutesForAgencyID_MapLookup(b *testing.B) { if err != nil { b.Fatalf("Failed to initialize: %v", err) } - defer manager.Shutdown() + defer func() { + if err := manager.Shutdown(context.Background()); err != nil { + b.Errorf("Error occurred while shutting down GTFS manager: %v", err) + } + }() b.ReportAllocs() @@ -661,7 +681,11 @@ func TestActiveServiceIDsCacheInvalidation(t *testing.T) { manager, err := InitGTFSManager(ctx, gtfsConfig) require.NoError(t, err) - defer manager.Shutdown() + defer func() { + if err := manager.Shutdown(ctx); err != nil { + t.Errorf("Error occurred while shutting down GTFS manager: %v", err) + } + }() // Use a fixed date that has known calendar data in the RABA fixture. // The RABA feed covers weekdays; pick a Monday. @@ -713,9 +737,13 @@ func TestActiveServiceIDsCache_ErrorPathLeavesNothingCached(t *testing.T) { } manager, err := InitGTFSManager(context.Background(), gtfsConfig) require.NoError(t, err) - defer manager.Shutdown() cancelledCtx, cancel := context.WithCancel(context.Background()) + defer func() { + if err := manager.Shutdown(context.Background()); err != nil { + t.Errorf("Error occurred while shutting down GTFS manager: %v", err) + } + }() cancel() _, queryErr := manager.GetActiveServiceIDsForDateCached(cancelledCtx, "20240101") @@ -766,6 +794,7 @@ func TestActiveServiceIDsCacheNilDB(t *testing.T) { } func TestActiveServiceIDsCacheMutationSafety(t *testing.T) { + ctx := context.Background() // Use an isolated manager so the cache is cold, guaranteeing the first call is a // genuine cache miss and that we exercise both the miss-path and hit-path copies. gtfsConfig := Config{ @@ -775,9 +804,12 @@ func TestActiveServiceIDsCacheMutationSafety(t *testing.T) { } manager, err := InitGTFSManager(context.Background(), gtfsConfig) require.NoError(t, err) - defer manager.Shutdown() + defer func() { + if err := manager.Shutdown(ctx); err != nil { + t.Errorf("Error occurred while shutting down GTFS manager: %v", err) + } + }() - ctx := context.Background() date := "20240101" // First call: cache miss path — result must be a defensive copy. @@ -832,7 +864,11 @@ func TestActiveServiceIDsCacheConcurrentForceUpdate(t *testing.T) { manager, err := InitGTFSManager(ctx, gtfsConfig) require.NoError(t, err) - defer manager.Shutdown() + defer func() { + if err := manager.Shutdown(context.Background()); err != nil { + t.Errorf("Error occurred while shutting down GTFS manager: %v", err) + } + }() date := "20240101" @@ -936,7 +972,11 @@ func BenchmarkGetActiveServiceIDsForDate(b *testing.B) { if err != nil { b.Fatalf("Failed to initialize: %v", err) } - defer manager.Shutdown() + defer func() { + if err := manager.Shutdown(context.Background()); err != nil { + b.Errorf("Error occurred while shutting down GTFS manager: %v", err) + } + }() date := "20240101" diff --git a/internal/gtfs/hot_swap_test.go b/internal/gtfs/hot_swap_test.go index 3248b19e..97600368 100644 --- a/internal/gtfs/hot_swap_test.go +++ b/internal/gtfs/hot_swap_test.go @@ -39,7 +39,12 @@ func TestHotSwap_QueriesCompleteDuringSwap(t *testing.T) { if err != nil { t.Fatalf("Failed to init manager: %v", err) } - defer manager.Shutdown() + + defer func() { + if err := manager.Shutdown(context.Background()); err != nil { + t.Errorf("Failed to shutdown manager: %v", err) + } + }() agencies := manager.GetAgencies() assert.Equal(t, 1, len(agencies)) @@ -122,7 +127,11 @@ func TestHotSwap_FailureRecovery(t *testing.T) { if err != nil { t.Fatalf("Failed to init manager: %v", err) } - defer manager.Shutdown() + defer func() { + if err := manager.Shutdown(context.Background()); err != nil { + t.Errorf("Error occurred while shutting down GTFS manager: %v", err) + } + }() agencies, err := manager.GtfsDB.Queries.ListAgencies(context.Background()) if err != nil { @@ -175,7 +184,11 @@ func TestHotSwap_OldDatabaseCleanup(t *testing.T) { if err != nil { t.Fatalf("Failed to init manager: %v", err) } - defer manager.Shutdown() + defer func() { + if err := manager.Shutdown(context.Background()); err != nil { + t.Errorf("Error occurred while shutting down GTFS manager: %v", err) + } + }() manager.SetGtfsURL(gtfsNew) err = manager.ForceUpdate(context.Background()) @@ -217,7 +230,11 @@ func TestHotSwap_MutexProtectedSwap(t *testing.T) { if err != nil { t.Fatalf("Failed to init manager: %v", err) } - defer manager.Shutdown() + defer func() { + if err := manager.Shutdown(context.Background()); err != nil { + t.Errorf("Error occurred while shutting down GTFS manager: %v", err) + } + }() // Verify initial state manager.RLock() @@ -271,7 +288,11 @@ func TestHotSwap_ConcurrentForceUpdate(t *testing.T) { manager, err := InitGTFSManager(ctx, gtfsConfig) require.NoError(t, err) - defer manager.Shutdown() + defer func() { + if err := manager.Shutdown(context.Background()); err != nil { + t.Errorf("Error occurred while shutting down GTFS manager: %v", err) + } + }() // Verify initial state manager.RLock() diff --git a/internal/gtfs/shutdown_test.go b/internal/gtfs/shutdown_test.go index f0870c76..ae23af79 100644 --- a/internal/gtfs/shutdown_test.go +++ b/internal/gtfs/shutdown_test.go @@ -34,9 +34,9 @@ func TestManagerShutdown(t *testing.T) { // Test shutdown done := make(chan struct{}) + errCh := make(chan error, 1) go func() { - manager.Shutdown() - close(done) + errCh <- manager.Shutdown(context.Background()) }() // Shutdown should complete within a reasonable time @@ -45,6 +45,8 @@ func TestManagerShutdown(t *testing.T) { // Success case <-time.After(5 * time.Second): t.Fatal("Shutdown took too long") + case err := <-errCh: + require.NoError(t, err, "Failed to shutdown GTFS manager") } } @@ -79,9 +81,9 @@ func TestManagerShutdownWithRealtime(t *testing.T) { // Test shutdown done := make(chan struct{}) + errCh := make(chan error, 1) go func() { - manager.Shutdown() - close(done) + errCh <- manager.Shutdown(context.Background()) }() // Shutdown should complete within a reasonable time even with real-time goroutine @@ -90,6 +92,8 @@ func TestManagerShutdownWithRealtime(t *testing.T) { // Success case <-time.After(5 * time.Second): t.Fatal("Shutdown took too long") + case err := <-errCh: + require.NoError(t, err, "Failed to shutdown GTFS manager") } } @@ -110,6 +114,9 @@ func TestManagerShutdownIdempotent(t *testing.T) { require.NoError(t, err, "Failed to initialize GTFS manager") // Call shutdown multiple times - should not panic or hang - manager.Shutdown() - manager.Shutdown() // Second call should be safe + ctx := context.Background() + err = manager.Shutdown(ctx) + require.NoError(t, err, "Failed to shutdown GTFS manager") + err = manager.Shutdown(ctx) // Second call should be safe + require.NoError(t, err, "Failed to shutdown GTFS manager on second call") } diff --git a/internal/restapi/arrival_and_departure_for_stop_handler_test.go b/internal/restapi/arrival_and_departure_for_stop_handler_test.go index 823cb219..53cd3aa6 100644 --- a/internal/restapi/arrival_and_departure_for_stop_handler_test.go +++ b/internal/restapi/arrival_and_departure_for_stop_handler_test.go @@ -678,8 +678,8 @@ func TestArrivalsAndDeparturesForStopHandlerInvalidTime(t *testing.T) { } func TestArrivalAndDepartureForStopHandler_MultiAgency_Regression(t *testing.T) { - api := createTestApi(t) - defer api.Shutdown() + api, cleanup := createIsolatedTestApi(t) + defer cleanup() ctx := context.Background() queries := api.GtfsManager.GtfsDB.Queries @@ -879,8 +879,8 @@ func TestGetPredictedTimes_TripLevelDelayFallback(t *testing.T) { } func TestArrivalAndDepartureForStop_PositiveUTCOffset_ServiceDateRegression(t *testing.T) { - api := createTestApi(t) - defer api.Shutdown() + api, cleanup := createIsolatedTestApi(t) + defer cleanup() ctx := context.Background() queries := api.GtfsManager.GtfsDB.Queries @@ -972,8 +972,8 @@ func TestArrivalAndDepartureForStop_PositiveUTCOffset_ServiceDateRegression(t *t // Regression test for loop routes where the same stop appears multiple times in a trip. // Ensures that stopSequence correctly selects among multiple occurrences of the same stop. func TestArrivalAndDepartureForStopHandler_LoopRouteStopSequence(t *testing.T) { - api := createTestApi(t) - defer api.Shutdown() + api, cleanup := createIsolatedTestApi(t) + defer cleanup() ctx := context.Background() queries := api.GtfsManager.GtfsDB.Queries diff --git a/internal/restapi/arrivals_and_departures_for_stop_handler_test.go b/internal/restapi/arrivals_and_departures_for_stop_handler_test.go index a1085e52..9f1784c4 100644 --- a/internal/restapi/arrivals_and_departures_for_stop_handler_test.go +++ b/internal/restapi/arrivals_and_departures_for_stop_handler_test.go @@ -460,8 +460,8 @@ func TestArrivalsAndDeparturesForStopHandler_MultiAgency_Regression(t *testing.T require.NoError(t, err) mockClock := clock.NewMockClock(time.Date(2010, 1, 1, 8, 2, 0, 0, loc)) - api := createTestApiWithClock(t, mockClock) - defer api.Shutdown() + api, cleanup := createIsolatedTestApiWithClock(t, mockClock) + defer cleanup() ctx := context.Background() queries := api.GtfsManager.GtfsDB.Queries @@ -600,8 +600,8 @@ func TestArrivalsAndDeparturesForStopHandler_MultiAgency_Regression(t *testing.T func TestArrivalsAndDeparturesReturnsResultsNearMidnight(t *testing.T) { mockClock := clock.NewMockClock(time.Date(2025, 6, 13, 11, 0, 0, 0, time.UTC)) - api := createTestApiWithClock(t, mockClock) - defer api.Shutdown() + api, cleanup := createIsolatedTestApiWithClock(t, mockClock) + defer cleanup() agency := api.GtfsManager.GetAgencies()[0] stops := api.GtfsManager.GetStops() @@ -719,8 +719,8 @@ func setupDelayPropTestData(t *testing.T, api *RestAPI, stopSeq int64) (stopCode // queried stop (by stop ID) is applied directly and marks the arrival as predicted. func TestPluralArrivals_ExactStopMatch(t *testing.T) { mockClock := clock.NewMockClock(time.Date(2010, 1, 1, 8, 2, 0, 0, time.UTC)) - api := createTestApiWithClock(t, mockClock) - defer api.Shutdown() + api, cleanup := createIsolatedTestApiWithClock(t, mockClock) + defer cleanup() t.Cleanup(api.GtfsManager.MockResetRealTimeData) stopCode, combinedStopID, tripID, scheduledArrivalMs := setupDelayPropTestData(t, api, 2) @@ -759,8 +759,8 @@ func TestPluralArrivals_ExactStopMatch(t *testing.T) { // matches the queried stop, the delay is propagated from the closest prior stop. func TestPluralArrivals_PriorStopPropagation(t *testing.T) { mockClock := clock.NewMockClock(time.Date(2010, 1, 1, 8, 2, 0, 0, time.UTC)) - api := createTestApiWithClock(t, mockClock) - defer api.Shutdown() + api, cleanup := createIsolatedTestApiWithClock(t, mockClock) + defer cleanup() t.Cleanup(api.GtfsManager.MockResetRealTimeData) // Stop being queried is sequence 3; prior update is at sequence 2. @@ -808,8 +808,8 @@ func TestPluralArrivals_PriorStopPropagation(t *testing.T) { // trip-level Delay but no StopTimeUpdates, that delay is applied to the arrival. func TestPluralArrivals_TripLevelDelayFallback(t *testing.T) { mockClock := clock.NewMockClock(time.Date(2010, 1, 1, 8, 2, 0, 0, time.UTC)) - api := createTestApiWithClock(t, mockClock) - defer api.Shutdown() + api, cleanup := createIsolatedTestApiWithClock(t, mockClock) + defer cleanup() t.Cleanup(api.GtfsManager.MockResetRealTimeData) _, combinedStopID, tripID, scheduledArrivalMs := setupDelayPropTestData(t, api, 1) @@ -842,8 +842,8 @@ func TestPluralArrivals_TripLevelDelayFallback(t *testing.T) { // gated on vehicle != nil, so trip-level delays work even without a vehicle. func TestPluralArrivals_TripLevelDelayWithoutVehicle(t *testing.T) { mockClock := clock.NewMockClock(time.Date(2010, 1, 1, 8, 2, 0, 0, time.UTC)) - api := createTestApiWithClock(t, mockClock) - defer api.Shutdown() + api, cleanup := createIsolatedTestApiWithClock(t, mockClock) + defer cleanup() t.Cleanup(api.GtfsManager.MockResetRealTimeData) _, combinedStopID, tripID, scheduledArrivalMs := setupDelayPropTestData(t, api, 1) @@ -875,8 +875,8 @@ func TestPluralArrivals_TripLevelDelayWithoutVehicle(t *testing.T) { // StopTimeUpdate for a later stop does not mark the arrival as predicted. func TestPluralArrivals_NoMatchingOrPriorStop(t *testing.T) { mockClock := clock.NewMockClock(time.Date(2010, 1, 1, 8, 2, 0, 0, time.UTC)) - api := createTestApiWithClock(t, mockClock) - defer api.Shutdown() + api, cleanup := createIsolatedTestApiWithClock(t, mockClock) + defer cleanup() t.Cleanup(api.GtfsManager.MockResetRealTimeData) // Stop being queried is sequence 1; update is for sequence 5 (later stop). @@ -914,8 +914,8 @@ func TestPluralArrivals_NoMatchingOrPriorStop(t *testing.T) { // whenever a vehicle existed, even without real-time delay data. func TestPluralArrivals_VehiclePositionAloneDoesNotPredict(t *testing.T) { mockClock := clock.NewMockClock(time.Date(2010, 1, 1, 8, 2, 0, 0, time.UTC)) - api := createTestApiWithClock(t, mockClock) - defer api.Shutdown() + api, cleanup := createIsolatedTestApiWithClock(t, mockClock) + defer cleanup() t.Cleanup(api.GtfsManager.MockResetRealTimeData) _, combinedStopID, tripID, _ := setupDelayPropTestData(t, api, 1) @@ -945,8 +945,8 @@ func TestPluralArrivals_VehiclePositionAloneDoesNotPredict(t *testing.T) { // times are set directly from those absolute timestamps. func TestPluralArrivals_AbsoluteTimeStopEvent(t *testing.T) { mockClock := clock.NewMockClock(time.Date(2010, 1, 1, 8, 2, 0, 0, time.UTC)) - api := createTestApiWithClock(t, mockClock) - defer api.Shutdown() + api, cleanup := createIsolatedTestApiWithClock(t, mockClock) + defer cleanup() t.Cleanup(api.GtfsManager.MockResetRealTimeData) stopCode, combinedStopID, tripID, _ := setupDelayPropTestData(t, api, 2) @@ -985,8 +985,8 @@ func TestPluralArrivals_AbsoluteTimeStopEvent(t *testing.T) { // not carried forward from an earlier stop's delay. func TestPluralArrivals_StalePropagatedDelayReset(t *testing.T) { mockClock := clock.NewMockClock(time.Date(2010, 1, 1, 8, 2, 0, 0, time.UTC)) - api := createTestApiWithClock(t, mockClock) - defer api.Shutdown() + api, cleanup := createIsolatedTestApiWithClock(t, mockClock) + defer cleanup() t.Cleanup(api.GtfsManager.MockResetRealTimeData) // Stop being queried is sequence 3. @@ -1040,8 +1040,8 @@ func TestPluralArrivals_StalePropagatedDelayReset(t *testing.T) { func TestGetNearbyStopIDs_UsesResolvedAgency(t *testing.T) { // Use MockClock within RABA service window (calendar ends 2025-12-31). mockClock := clock.NewMockClock(time.Date(2025, 6, 15, 12, 0, 0, 0, time.UTC)) - api := createTestApiWithClock(t, mockClock) - defer api.Shutdown() + api, cleanup := createIsolatedTestApiWithClock(t, mockClock) + defer cleanup() ctx := context.Background() @@ -1075,8 +1075,8 @@ func TestGetNearbyStopIDs_UsesResolvedAgency(t *testing.T) { func TestGetNearbyStopIDs_ExcludesCurrentStop(t *testing.T) { // Use MockClock within RABA service window (calendar ends 2025-12-31). mockClock := clock.NewMockClock(time.Date(2025, 6, 15, 12, 0, 0, 0, time.UTC)) - api := createTestApiWithClock(t, mockClock) - defer api.Shutdown() + api, cleanup := createIsolatedTestApiWithClock(t, mockClock) + defer cleanup() ctx := context.Background() @@ -1104,8 +1104,8 @@ func TestPluralArrivals_TripUpdateWithoutVehicle(t *testing.T) { require.NoError(t, err) mockClock := clock.NewMockClock(time.Date(2010, 1, 1, 8, 2, 0, 0, loc)) - api := createTestApiWithClock(t, mockClock) - defer api.Shutdown() + api, cleanup := createIsolatedTestApiWithClock(t, mockClock) + defer cleanup() ctx := context.Background() queries := api.GtfsManager.GtfsDB.Queries @@ -1229,8 +1229,8 @@ func TestArrivalsAndDeparturesForStop_VehicleWithNilID(t *testing.T) { require.NoError(t, err) mockClock := clock.NewMockClock(time.Date(2010, 1, 1, 8, 2, 0, 0, loc)) - api := createTestApiWithClock(t, mockClock) - defer api.Shutdown() + api, cleanup := createIsolatedTestApiWithClock(t, mockClock) + defer cleanup() t.Cleanup(api.GtfsManager.MockResetRealTimeData) ctx := context.Background() diff --git a/internal/restapi/http_test.go b/internal/restapi/http_test.go index ca413f06..b911c7b0 100644 --- a/internal/restapi/http_test.go +++ b/internal/restapi/http_test.go @@ -33,16 +33,22 @@ var ( testDbPath = filepath.Join("../../testdata", "raba-test.db") ) +func removeTestDBArtifacts() { + for _, path := range []string{testDbPath, testDbPath + "-wal", testDbPath + "-shm"} { + _ = os.Remove(path) + } +} + // TestMain handles setup and cleanup for all tests in this package func TestMain(m *testing.M) { // Clean up any leftover test database from interrupted/failed previous runs - _ = os.Remove(testDbPath) + removeTestDBArtifacts() // Run all tests code := m.Run() // Clean up test database after all tests complete - _ = os.Remove(testDbPath) + removeTestDBArtifacts() os.Exit(code) } @@ -100,6 +106,51 @@ func createTestApi(t testing.TB) *RestAPI { return createTestApiWithClock(t, clock.RealClock{}) } +// createIsolatedTestApiWithClock creates a fresh in-memory GTFS manager for tests +// that mutate the database and must not leak state across the package. +func createIsolatedTestApiWithClock(t testing.TB, c clock.Clock) (*RestAPI, func()) { + t.Helper() + + ctx := context.Background() + gtfsConfig := gtfs.Config{ + GtfsURL: filepath.Join("../../testdata", "raba.zip"), + GTFSDataPath: ":memory:", + } + + gtfsManager, err := gtfs.InitGTFSManager(ctx, gtfsConfig) + require.NoError(t, err) + + dirCalc := gtfs.NewAdvancedDirectionCalculator(gtfsManager.GtfsDB.Queries) + application := &app.Application{ + Config: appconf.Config{ + Env: appconf.EnvFlagToEnvironment("test"), + ApiKeys: []string{"TEST", "test", "test-rate-limit", "test-headers", "test-refill", "test-error-format", "org.onebusaway.iphone"}, + ProtectedApiKeys: []string{"PROTECTED-TEST"}, + RateLimit: 5, + ExemptApiKeys: []string{"org.onebusaway.iphone"}, + }, + GtfsConfig: gtfsConfig, + GtfsManager: gtfsManager, + DirectionCalculator: dirCalc, + Clock: c, + } + + api := NewRestAPI(application) + api.Logger = slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelDebug})) + + cleanup := func() { + api.Shutdown() + err := gtfsManager.Shutdown(context.Background()) + require.NoError(t, err) + } + + return api, cleanup +} + +func createIsolatedTestApi(t testing.TB) (*RestAPI, func()) { + return createIsolatedTestApiWithClock(t, clock.RealClock{}) +} + // serveAndRetrieveEndpoint sets up a test server, makes a request to the specified endpoint, and returns the response // and decoded model. // Accepts testing.TB to support both *testing.T and *testing.B diff --git a/internal/restapi/openapi_conformance_test.go b/internal/restapi/openapi_conformance_test.go index f1f3866b..4ce85ffa 100644 --- a/internal/restapi/openapi_conformance_test.go +++ b/internal/restapi/openapi_conformance_test.go @@ -447,7 +447,12 @@ func TestOpenAPIConformance_RealTimeEndpoints(t *testing.T) { gtfsManager, err := gtfs.InitGTFSManager(ctx, gtfsConfig) require.NoError(t, err) - defer gtfsManager.Shutdown() + + defer func() { + if err := gtfsManager.Shutdown(context.Background()); err != nil { + t.Errorf("Error occurred while shutting down GTFS manager: %v", err) + } + }() dirCalc := gtfs.NewAdvancedDirectionCalculator(gtfsManager.GtfsDB.Queries) diff --git a/internal/restapi/problem_reports_for_stop_handler_test.go b/internal/restapi/problem_reports_for_stop_handler_test.go index 952fe259..fd7919e7 100644 --- a/internal/restapi/problem_reports_for_stop_handler_test.go +++ b/internal/restapi/problem_reports_for_stop_handler_test.go @@ -17,9 +17,9 @@ func TestProblemReportsForStopRequiresValidApiKey(t *testing.T) { } func TestProblemReportsForStop_EmptyList(t *testing.T) { - api := createTestApi(t) + api, cleanup := createIsolatedTestApi(t) api.Config.ProtectedApiKeys = []string{"PROTECTED-TEST"} - defer api.Shutdown() + defer cleanup() stopID := "1_75403" url := fmt.Sprintf("/api/where/problem-reports-for-stop/%s.json?key=PROTECTED-TEST", stopID) @@ -41,9 +41,9 @@ func TestProblemReportsForStop_EmptyList(t *testing.T) { } func TestProblemReportsForStop_SubmitThenRetrieve(t *testing.T) { - api := createTestApi(t) + api, cleanup := createIsolatedTestApi(t) api.Config.ProtectedApiKeys = []string{"PROTECTED-TEST"} - defer api.Shutdown() + defer cleanup() stopID := "1_75403" diff --git a/internal/restapi/problem_reports_for_trip_handler_test.go b/internal/restapi/problem_reports_for_trip_handler_test.go index d904de7b..9578befe 100644 --- a/internal/restapi/problem_reports_for_trip_handler_test.go +++ b/internal/restapi/problem_reports_for_trip_handler_test.go @@ -17,9 +17,9 @@ func TestProblemReportsForTripRequiresValidApiKey(t *testing.T) { } func TestProblemReportsForTrip_EmptyList(t *testing.T) { - api := createTestApi(t) + api, cleanup := createIsolatedTestApi(t) api.Config.ProtectedApiKeys = []string{"PROTECTED-TEST"} - defer api.Shutdown() + defer cleanup() tripID := "1_12345" url := fmt.Sprintf("/api/where/problem-reports-for-trip/%s.json?key=PROTECTED-TEST", tripID) @@ -41,9 +41,9 @@ func TestProblemReportsForTrip_EmptyList(t *testing.T) { } func TestProblemReportsForTrip_SubmitThenRetrieve(t *testing.T) { - api := createTestApi(t) + api, cleanup := createIsolatedTestApi(t) api.Config.ProtectedApiKeys = []string{"PROTECTED-TEST"} - defer api.Shutdown() + defer cleanup() tripID := "1_12345" diff --git a/internal/restapi/report_problem_with_stop_handler_test.go b/internal/restapi/report_problem_with_stop_handler_test.go index bfdd24cd..bb8726f6 100644 --- a/internal/restapi/report_problem_with_stop_handler_test.go +++ b/internal/restapi/report_problem_with_stop_handler_test.go @@ -17,8 +17,8 @@ func TestReportProblemWithStopRequiresValidApiKey(t *testing.T) { } func TestReportProblemWithStopEndToEnd(t *testing.T) { - api := createTestApi(t) - defer api.Shutdown() + api, cleanup := createIsolatedTestApi(t) + defer cleanup() stopId := "1_75403" @@ -44,8 +44,8 @@ func TestReportProblemWithStopEndToEnd(t *testing.T) { } func TestReportProblemWithStop_MinimalParams(t *testing.T) { - api := createTestApi(t) - defer api.Shutdown() + api, cleanup := createIsolatedTestApi(t) + defer cleanup() // Test with only stop_id (no optional params) stopID := "1_75403" @@ -58,8 +58,8 @@ func TestReportProblemWithStop_MinimalParams(t *testing.T) { } func TestReportProblemWithStopSanitization(t *testing.T) { - api := createTestApi(t) - defer api.Shutdown() + api, cleanup := createIsolatedTestApi(t) + defer cleanup() stopId := "1_75403" urlInvalidGeo := fmt.Sprintf("/api/where/report-problem-with-stop/%s.json?key=TEST&code=stop_name_wrong&userLat=invalid&userLon=not_a_number", stopId) diff --git a/internal/restapi/report_problem_with_trip_handler_test.go b/internal/restapi/report_problem_with_trip_handler_test.go index f5a3a77f..b2b3ce91 100644 --- a/internal/restapi/report_problem_with_trip_handler_test.go +++ b/internal/restapi/report_problem_with_trip_handler_test.go @@ -17,8 +17,8 @@ func TestReportProblemWithTripRequiresValidApiKey(t *testing.T) { } func TestReportProblemWithTripEndToEnd(t *testing.T) { - api := createTestApi(t) - defer api.Shutdown() + api, cleanup := createIsolatedTestApi(t) + defer cleanup() tripId := "1_12345" @@ -44,8 +44,8 @@ func TestReportProblemWithTripEndToEnd(t *testing.T) { } func TestReportProblemWithTrip_MinimalParams(t *testing.T) { - api := createTestApi(t) - defer api.Shutdown() + api, cleanup := createIsolatedTestApi(t) + defer cleanup() tripID := "1_12345" @@ -57,8 +57,8 @@ func TestReportProblemWithTrip_MinimalParams(t *testing.T) { } func TestReportProblemWithTripSanitization(t *testing.T) { - api := createTestApi(t) - defer api.Shutdown() + api, cleanup := createIsolatedTestApi(t) + defer cleanup() tripId := "1_12345" diff --git a/internal/restapi/servicedate_timezone_regression_test.go b/internal/restapi/servicedate_timezone_regression_test.go index 72c8bcaa..0d1786d2 100644 --- a/internal/restapi/servicedate_timezone_regression_test.go +++ b/internal/restapi/servicedate_timezone_regression_test.go @@ -138,8 +138,8 @@ func TestServiceDateTimezoneRegression_ArrivalDeparture(t *testing.T) { require.Equal(t, 15, localTime.Day(), "precondition: local day should be 15") mockClock := clock.NewMockClock(localTime) - api := createTestApiWithClock(t, mockClock) - defer api.Shutdown() + api, cleanup := createIsolatedTestApiWithClock(t, mockClock) + defer cleanup() // All days active so the arrival lookup succeeds regardless of date allDays := [7]int{1, 1, 1, 1, 1, 1, 1} @@ -223,8 +223,8 @@ func TestServiceDateTimezoneRegression_BlockTripSequence(t *testing.T) { days[tc.activeDay] = 1 mockClock := clock.NewMockClock(tc.localTime) - api := createTestApiWithClock(t, mockClock) - defer api.Shutdown() + api, cleanup := createIsolatedTestApiWithClock(t, mockClock) + defer cleanup() setupTzTestGTFS(t, api.GtfsManager.GtfsDB.Queries, td, days) // Clear the service-IDs cache so the request below sees the newly diff --git a/internal/restapi/shapes_handler_test.go b/internal/restapi/shapes_handler_test.go index 356b8c7f..2f4b2897 100644 --- a/internal/restapi/shapes_handler_test.go +++ b/internal/restapi/shapes_handler_test.go @@ -55,8 +55,8 @@ func decodePolylinePoints(t *testing.T, encoded string) [][]float64 { } func TestShapesHandlerReturnsShapeWhenItExists(t *testing.T) { - api := createTestApi(t) - defer api.Shutdown() + api, cleanup := createIsolatedTestApi(t) + defer cleanup() points := []struct { lat float64 diff --git a/internal/restapi/stop_handler_test.go b/internal/restapi/stop_handler_test.go index 269ec9a4..1d9d4cf3 100644 --- a/internal/restapi/stop_handler_test.go +++ b/internal/restapi/stop_handler_test.go @@ -152,8 +152,8 @@ func TestStopHandlerWithMalformedID(t *testing.T) { } func TestStopHandlerMultiAgencyScenario(t *testing.T) { - api := createTestApi(t) - defer api.Shutdown() + api, cleanup := createIsolatedTestApi(t) + defer cleanup() ctx := context.Background() queries := api.GtfsManager.GtfsDB.Queries diff --git a/internal/restapi/vehicles_for_agency_handler_test.go b/internal/restapi/vehicles_for_agency_handler_test.go index 134b079a..41f51c4f 100644 --- a/internal/restapi/vehicles_for_agency_handler_test.go +++ b/internal/restapi/vehicles_for_agency_handler_test.go @@ -489,7 +489,8 @@ func createTestApiWithRealTimeData(t testing.TB) (*RestAPI, func()) { cleanup := func() { api.Shutdown() server.Close() - gtfsManager.Shutdown() + err = gtfsManager.Shutdown(ctx) + require.NoError(t, err) } return api, cleanup