From 1642a1ae76ea12c8a22b913f135f38957f5ff60e Mon Sep 17 00:00:00 2001 From: Shawn Reuland Date: Wed, 17 Jul 2024 17:32:22 -0700 Subject: [PATCH] #4911: fix integeation test StartHorizon usage when intentional cmd exec error, make sure close db conn regardless during test shutdown --- .../internal/integration/parameters_test.go | 27 ------------------- .../internal/test/integration/integration.go | 13 ++++++++- 2 files changed, 12 insertions(+), 28 deletions(-) diff --git a/services/horizon/internal/integration/parameters_test.go b/services/horizon/internal/integration/parameters_test.go index 3d131ba77d..91e16ce186 100644 --- a/services/horizon/internal/integration/parameters_test.go +++ b/services/horizon/internal/integration/parameters_test.go @@ -12,7 +12,6 @@ import ( "strings" "sync" "testing" - "time" "github.com/spf13/cobra" @@ -80,9 +79,6 @@ func TestBucketDirDisallowed(t *testing.T) { assert.Equal(t, err.Error(), integration.HorizonInitErrStr+": error generating captive core configuration:"+ " invalid captive core toml file: could not unmarshal captive core toml: setting BUCKET_DIR_PATH is disallowed"+ " for Captive Core, use CAPTIVE_CORE_STORAGE_PATH instead") - time.Sleep(1 * time.Second) - test.StopHorizon() - test.Shutdown() } func TestEnvironmentPreserved(t *testing.T) { @@ -117,8 +113,6 @@ func TestEnvironmentPreserved(t *testing.T) { envValue := os.Getenv("STELLAR_CORE_URL") assert.Equal(t, StellarCoreURL, envValue) - test.Shutdown() - envValue = os.Getenv("STELLAR_CORE_URL") assert.Equal(t, "original value", envValue) } @@ -161,11 +155,7 @@ func TestInvalidNetworkParameters(t *testing.T) { testConfig.HorizonIngestParameters = localParams test := integration.NewTest(t, *testConfig) err := test.StartHorizon(true) - // Adding sleep as a workaround for the race condition in the ingestion system. - // https://github.com/stellar/go/issues/5005 - time.Sleep(2 * time.Second) assert.Equal(t, testCase.errMsg, err.Error()) - test.Shutdown() }) } } @@ -205,14 +195,9 @@ func TestNetworkParameter(t *testing.T) { testConfig.HorizonIngestParameters = localParams test := integration.NewTest(t, *testConfig) err := test.StartHorizon(true) - // Adding sleep as a workaround for the race condition in the ingestion system. - // https://github.com/stellar/go/issues/5005 - time.Sleep(2 * time.Second) assert.NoError(t, err) assert.Equal(t, test.GetHorizonIngestConfig().HistoryArchiveURLs, tt.historyArchiveURLs) assert.Equal(t, test.GetHorizonIngestConfig().NetworkPassphrase, tt.networkPassphrase) - - test.Shutdown() }) } } @@ -248,11 +233,7 @@ func TestNetworkEnvironmentVariable(t *testing.T) { testConfig.HorizonEnvironment = map[string]string{"NETWORK": networkValue} test := integration.NewTest(t, *testConfig) err := test.StartHorizon(true) - // Adding sleep here as a workaround for the race condition in the ingestion system. - // More details can be found at https://github.com/stellar/go/issues/5005 - time.Sleep(2 * time.Second) assert.NoError(t, err) - test.Shutdown() }) } } @@ -300,7 +281,6 @@ func TestMaxAssetsForPathRequests(t *testing.T) { assert.NoError(t, err) test.WaitForHorizonIngest() assert.Equal(t, test.HorizonIngest().Config().MaxAssetsPerPathRequest, 2) - test.Shutdown() }) } @@ -326,7 +306,6 @@ func TestMaxPathFindingRequests(t *testing.T) { finder, ok := test.HorizonIngest().Paths().(*paths.RateLimitedFinder) assert.True(t, ok) assert.Equal(t, finder.Limit(), 5) - test.Shutdown() }) } @@ -339,7 +318,6 @@ func TestDisablePathFinding(t *testing.T) { assert.Equal(t, test.HorizonIngest().Config().MaxPathFindingRequests, uint(0)) _, ok := test.HorizonIngest().Paths().(simplepath.InMemoryFinder) assert.True(t, ok) - test.Shutdown() }) t.Run("set to true", func(t *testing.T) { testConfig := integration.GetTestConfig() @@ -349,7 +327,6 @@ func TestDisablePathFinding(t *testing.T) { assert.NoError(t, err) test.WaitForHorizonIngest() assert.Nil(t, test.HorizonIngest().Paths()) - test.Shutdown() }) } @@ -366,7 +343,6 @@ func TestDisableTxSub(t *testing.T) { test := integration.NewTest(t, *testConfig) err := test.StartHorizon(true) assert.ErrorContains(t, err, "cannot initialize Horizon: flag --stellar-core-url cannot be empty") - test.Shutdown() }) t.Run("horizon starts successfully when DISABLE_TX_SUB=false, INGEST=false and stellar-core-url is provided", func(t *testing.T) { localParams := integration.MergeMaps(networkParamArgs, map[string]string{ @@ -381,7 +357,6 @@ func TestDisableTxSub(t *testing.T) { test := integration.NewTest(t, *testConfig) err := test.StartHorizon(true) assert.NoError(t, err) - test.Shutdown() }) t.Run("horizon starts successfully when DISABLE_TX_SUB=true and INGEST=true", func(t *testing.T) { testConfig := integration.GetTestConfig() @@ -393,7 +368,6 @@ func TestDisableTxSub(t *testing.T) { err := test.StartHorizon(true) assert.NoError(t, err) test.WaitForHorizonIngest() - test.Shutdown() }) t.Run("do not require stellar-core-url when both DISABLE_TX_SUB=true and INGEST=false", func(t *testing.T) { localParams := integration.MergeMaps(networkParamArgs, map[string]string{ @@ -407,7 +381,6 @@ func TestDisableTxSub(t *testing.T) { test := integration.NewTest(t, *testConfig) err := test.StartHorizon(true) assert.NoError(t, err) - test.Shutdown() }) } diff --git a/services/horizon/internal/test/integration/integration.go b/services/horizon/internal/test/integration/integration.go index a4fcf34469..ca99fa4c06 100644 --- a/services/horizon/internal/test/integration/integration.go +++ b/services/horizon/internal/test/integration/integration.go @@ -342,6 +342,18 @@ func (i *Test) Shutdown() { func (i *Test) StartHorizon(startIngestProcess bool) error { i.testDB = dbtest.Postgres(i.t) i.shutdownCalls = append(i.shutdownCalls, func() { + if i.appStopped == nil { + // appStopped is nil when the horizon cmd.Execute creates an App, but gets an intentional error and StartHorizon + // never gets to point of running App.Serve() which would have closed the db conn eventually + // since it wires up listener to App.Close() invocation, so, we must manually detect this edge case and + // close the app's db here to clean up + if i.webNode != nil { + i.webNode.CloseDB() + } + if i.ingestNode != nil { + i.ingestNode.CloseDB() + } + } i.StopHorizon() i.testDB.Close() }) @@ -401,7 +413,6 @@ func (i *Test) StartHorizon(startIngestProcess bool) error { } i.appStopped = &sync.WaitGroup{} - if i.ingestNode != nil { i.appStopped.Add(1) go func() {