From eda07614ce2e08fffc039276af1f9963beef7c8a Mon Sep 17 00:00:00 2001 From: andres-portainer <91705312+andres-portainer@users.noreply.github.com> Date: Tue, 30 May 2023 11:02:22 -0300 Subject: [PATCH] chore(unit-test): simplify teardown EE-5536 (#9015) --- api/apikey/service_test.go | 21 ++--- api/dataservices/stack/tests/stack_test.go | 6 +- api/dataservices/team/tests/team_test.go | 9 +-- api/datastore/backup_test.go | 13 +-- api/datastore/datastore_test.go | 3 +- api/datastore/migrate_data_test.go | 8 +- api/datastore/migrate_dbversion29_test.go | 23 +++--- api/datastore/migrate_dbversion33_test.go | 3 +- api/datastore/teststore.go | 6 +- .../customtemplate_git_fetch_test.go | 3 +- .../edgestacks/edgestack_create_test.go | 6 +- .../edgestacks/edgestack_delete_test.go | 6 +- .../edgestacks/edgestack_inspect_test.go | 3 +- .../edgestack_status_delete_test.go | 3 +- .../edgestack_status_update_test.go | 6 +- api/http/handler/edgestacks/edgestack_test.go | 10 +-- .../edgestacks/edgestack_update_test.go | 9 +-- .../endpointedge_status_inspect_test.go | 80 +++++-------------- .../handler/endpoints/endpoint_delete_test.go | 3 +- .../handler/endpoints/endpoint_list_test.go | 19 ++--- api/http/handler/endpoints/filter_test.go | 16 ++-- .../utils_update_edge_groups_test.go | 4 +- .../endpoints/utils_update_tags_test.go | 4 +- api/http/handler/helm/helm_delete_test.go | 3 +- api/http/handler/helm/helm_install_test.go | 3 +- api/http/handler/helm/helm_list_test.go | 3 +- .../handler/stacks/webhook_invoke_test.go | 3 +- api/http/handler/tags/tag_delete_test.go | 3 +- api/http/handler/teams/team_list_test.go | 3 +- .../users/user_create_access_token_test.go | 3 +- api/http/handler/users/user_delete_test.go | 3 +- .../users/user_get_access_tokens_test.go | 3 +- api/http/handler/users/user_list_test.go | 3 +- .../users/user_remove_access_token_test.go | 3 +- api/http/handler/users/user_update_test.go | 3 +- api/http/security/bouncer_test.go | 6 +- api/stacks/deployments/deploy_test.go | 22 ++--- 37 files changed, 110 insertions(+), 218 deletions(-) diff --git a/api/apikey/service_test.go b/api/apikey/service_test.go index 707bb73c3..a9a82d43d 100644 --- a/api/apikey/service_test.go +++ b/api/apikey/service_test.go @@ -22,8 +22,7 @@ func Test_SatisfiesAPIKeyServiceInterface(t *testing.T) { func Test_GenerateApiKey(t *testing.T) { is := assert.New(t) - _, store, teardown := datastore.MustNewTestStore(t, true, true) - defer teardown() + _, store := datastore.MustNewTestStore(t, true, true) service := NewAPIKeyService(store.APIKeyRepository(), store.User()) @@ -76,8 +75,7 @@ func Test_GenerateApiKey(t *testing.T) { func Test_GetAPIKey(t *testing.T) { is := assert.New(t) - _, store, teardown := datastore.MustNewTestStore(t, true, true) - defer teardown() + _, store := datastore.MustNewTestStore(t, true, true) service := NewAPIKeyService(store.APIKeyRepository(), store.User()) @@ -96,8 +94,7 @@ func Test_GetAPIKey(t *testing.T) { func Test_GetAPIKeys(t *testing.T) { is := assert.New(t) - _, store, teardown := datastore.MustNewTestStore(t, true, true) - defer teardown() + _, store := datastore.MustNewTestStore(t, true, true) service := NewAPIKeyService(store.APIKeyRepository(), store.User()) @@ -117,8 +114,7 @@ func Test_GetAPIKeys(t *testing.T) { func Test_GetDigestUserAndKey(t *testing.T) { is := assert.New(t) - _, store, teardown := datastore.MustNewTestStore(t, true, true) - defer teardown() + _, store := datastore.MustNewTestStore(t, true, true) service := NewAPIKeyService(store.APIKeyRepository(), store.User()) @@ -153,8 +149,7 @@ func Test_GetDigestUserAndKey(t *testing.T) { func Test_UpdateAPIKey(t *testing.T) { is := assert.New(t) - _, store, teardown := datastore.MustNewTestStore(t, true, true) - defer teardown() + _, store := datastore.MustNewTestStore(t, true, true) service := NewAPIKeyService(store.APIKeyRepository(), store.User()) @@ -199,8 +194,7 @@ func Test_UpdateAPIKey(t *testing.T) { func Test_DeleteAPIKey(t *testing.T) { is := assert.New(t) - _, store, teardown := datastore.MustNewTestStore(t, true, true) - defer teardown() + _, store := datastore.MustNewTestStore(t, true, true) service := NewAPIKeyService(store.APIKeyRepository(), store.User()) @@ -240,8 +234,7 @@ func Test_DeleteAPIKey(t *testing.T) { func Test_InvalidateUserKeyCache(t *testing.T) { is := assert.New(t) - _, store, teardown := datastore.MustNewTestStore(t, true, true) - defer teardown() + _, store := datastore.MustNewTestStore(t, true, true) service := NewAPIKeyService(store.APIKeyRepository(), store.User()) diff --git a/api/dataservices/stack/tests/stack_test.go b/api/dataservices/stack/tests/stack_test.go index 08106e625..0929c3087 100644 --- a/api/dataservices/stack/tests/stack_test.go +++ b/api/dataservices/stack/tests/stack_test.go @@ -29,8 +29,7 @@ func TestService_StackByWebhookID(t *testing.T) { if testing.Short() { t.Skip("skipping test in short mode. Normally takes ~1s to run.") } - _, store, teardown := datastore.MustNewTestStore(t, true, true) - defer teardown() + _, store := datastore.MustNewTestStore(t, true, true) b := stackBuilder{t: t, store: store} b.createNewStack(newGuidString(t)) @@ -87,8 +86,7 @@ func Test_RefreshableStacks(t *testing.T) { if testing.Short() { t.Skip("skipping test in short mode. Normally takes ~1s to run.") } - _, store, teardown := datastore.MustNewTestStore(t, true, true) - defer teardown() + _, store := datastore.MustNewTestStore(t, true, true) staticStack := portainer.Stack{ID: 1} stackWithWebhook := portainer.Stack{ID: 2, AutoUpdate: &portainer.AutoUpdateSettings{Webhook: "webhook"}} diff --git a/api/dataservices/team/tests/team_test.go b/api/dataservices/team/tests/team_test.go index cf0ea77f1..ae2bec576 100644 --- a/api/dataservices/team/tests/team_test.go +++ b/api/dataservices/team/tests/team_test.go @@ -10,8 +10,7 @@ import ( func Test_teamByName(t *testing.T) { t.Run("When store is empty should return ErrObjectNotFound", func(t *testing.T) { - _, store, teardown := datastore.MustNewTestStore(t, true, true) - defer teardown() + _, store := datastore.MustNewTestStore(t, true, true) _, err := store.Team().TeamByName("name") assert.ErrorIs(t, err, errors.ErrObjectNotFound) @@ -19,8 +18,7 @@ func Test_teamByName(t *testing.T) { }) t.Run("When there is no object with the same name should return ErrObjectNotFound", func(t *testing.T) { - _, store, teardown := datastore.MustNewTestStore(t, true, true) - defer teardown() + _, store := datastore.MustNewTestStore(t, true, true) teamBuilder := teamBuilder{ t: t, @@ -35,8 +33,7 @@ func Test_teamByName(t *testing.T) { }) t.Run("When there is an object with the same name should return the object", func(t *testing.T) { - _, store, teardown := datastore.MustNewTestStore(t, true, true) - defer teardown() + _, store := datastore.MustNewTestStore(t, true, true) teamBuilder := teamBuilder{ t: t, diff --git a/api/datastore/backup_test.go b/api/datastore/backup_test.go index 4fab6c564..f93103859 100644 --- a/api/datastore/backup_test.go +++ b/api/datastore/backup_test.go @@ -11,8 +11,7 @@ import ( ) func TestCreateBackupFolders(t *testing.T) { - _, store, teardown := MustNewTestStore(t, true, true) - defer teardown() + _, store := MustNewTestStore(t, true, true) connection := store.GetConnection() backupPath := path.Join(connection.GetStorePath(), backupDefaults.backupDir) @@ -28,9 +27,7 @@ func TestCreateBackupFolders(t *testing.T) { } func TestStoreCreation(t *testing.T) { - _, store, teardown := MustNewTestStore(t, true, true) - defer teardown() - + _, store := MustNewTestStore(t, true, true) if store == nil { t.Error("Expect to create a store") } @@ -41,9 +38,8 @@ func TestStoreCreation(t *testing.T) { } func TestBackup(t *testing.T) { - _, store, teardown := MustNewTestStore(t, true, true) + _, store := MustNewTestStore(t, true, true) connection := store.GetConnection() - defer teardown() t.Run("Backup should create default db backup", func(t *testing.T) { v := models.Version{ @@ -71,8 +67,7 @@ func TestBackup(t *testing.T) { } func TestRemoveWithOptions(t *testing.T) { - _, store, teardown := MustNewTestStore(t, true, true) - defer teardown() + _, store := MustNewTestStore(t, true, true) t.Run("successfully removes file if existent", func(t *testing.T) { store.createBackupFolders() diff --git a/api/datastore/datastore_test.go b/api/datastore/datastore_test.go index 1657d73f7..fe9c3012e 100644 --- a/api/datastore/datastore_test.go +++ b/api/datastore/datastore_test.go @@ -27,8 +27,7 @@ const ( // TestStoreFull an eventually comprehensive set of tests for the Store. // The idea is what we write to the store, we should read back. func TestStoreFull(t *testing.T) { - _, store, teardown := MustNewTestStore(t, true, true) - defer teardown() + _, store := MustNewTestStore(t, true, true) testCases := map[string]func(t *testing.T){ "User Accounts": func(t *testing.T) { diff --git a/api/datastore/migrate_data_test.go b/api/datastore/migrate_data_test.go index 81e175c59..7917bcd83 100644 --- a/api/datastore/migrate_data_test.go +++ b/api/datastore/migrate_data_test.go @@ -163,8 +163,7 @@ func TestMigrateData(t *testing.T) { } func Test_getBackupRestoreOptions(t *testing.T) { - _, store, teardown := MustNewTestStore(t, false, true) - defer teardown() + _, store := MustNewTestStore(t, false, true) options := getBackupRestoreOptions(store.commonBackupDir()) @@ -182,8 +181,7 @@ func Test_getBackupRestoreOptions(t *testing.T) { func TestRollback(t *testing.T) { t.Run("Rollback should restore upgrade after backup", func(t *testing.T) { version := models.Version{SchemaVersion: "2.4.0"} - _, store, teardown := MustNewTestStore(t, true, false) - defer teardown() + _, store := MustNewTestStore(t, true, false) err := store.VersionService.UpdateVersion(&version) if err != nil { @@ -240,7 +238,7 @@ func migrateDBTestHelper(t *testing.T, srcPath, wantPath string, overrideInstanc // Parse source json to db. // When we create a new test store, it sets its version field automatically to latest. - _, store, _ := MustNewTestStore(t, true, false) + _, store := MustNewTestStore(t, true, false) fmt.Println("store.path=", store.GetConnection().GetDatabaseFilePath()) store.connection.DeleteObject("version", []byte("VERSION")) diff --git a/api/datastore/migrate_dbversion29_test.go b/api/datastore/migrate_dbversion29_test.go index 0cde4c80f..4d1130faa 100644 --- a/api/datastore/migrate_dbversion29_test.go +++ b/api/datastore/migrate_dbversion29_test.go @@ -14,27 +14,19 @@ const dummyLogoURL = "example.com" // for unit testing usage only since using NewStore will cause cycle import inside migrator pkg func initTestingSettingsService(dbConn portainer.Connection, preSetObj map[string]interface{}) error { //insert a obj - if err := dbConn.UpdateObject("settings", []byte("SETTINGS"), preSetObj); err != nil { - return err - } - return nil + return dbConn.UpdateObject("settings", []byte("SETTINGS"), preSetObj) } func setup(store *Store) error { - var err error dummySettingsObj := map[string]interface{}{ "LogoURL": dummyLogoURL, } - err = initTestingSettingsService(store.connection, dummySettingsObj) - if err != nil { - return err - } - return nil + + return initTestingSettingsService(store.connection, dummySettingsObj) } func TestMigrateSettings(t *testing.T) { - _, store, teardown := MustNewTestStore(t, false, true) - defer teardown() + _, store := MustNewTestStore(t, false, true) err := setup(store) if err != nil { @@ -46,9 +38,11 @@ func TestMigrateSettings(t *testing.T) { if updatedSettings.LogoURL != dummyLogoURL { // ensure a pre-migrate setting isn't unset t.Errorf("unexpected value changes in the updated settings, want LogoURL value: %s, got LogoURL value: %s", dummyLogoURL, updatedSettings.LogoURL) } + if updatedSettings.OAuthSettings.SSO != false { // I recon golang defaulting will make this false t.Errorf("unexpected default OAuth SSO setting, want: false, got: %t", updatedSettings.OAuthSettings.SSO) } + if updatedSettings.OAuthSettings.LogoutURI != "" { t.Errorf("unexpected default OAuth HideInternalAuth setting, want:, got: %s", updatedSettings.OAuthSettings.LogoutURI) } @@ -72,18 +66,23 @@ func TestMigrateSettings(t *testing.T) { DockerhubService: store.DockerHubService, AuthorizationService: authorization.NewService(store), }) + if err := m.MigrateSettingsToDB30(); err != nil { t.Errorf("failed to update settings: %v", err) } + if err != nil { t.Errorf("failed to retrieve the updated settings: %v", err) } + if updatedSettings.LogoURL != dummyLogoURL { t.Errorf("unexpected value changes in the updated settings, want LogoURL value: %s, got LogoURL value: %s", dummyLogoURL, updatedSettings.LogoURL) } + if updatedSettings.OAuthSettings.SSO != false { t.Errorf("unexpected default OAuth SSO setting, want: false, got: %t", updatedSettings.OAuthSettings.SSO) } + if updatedSettings.OAuthSettings.LogoutURI != "" { t.Errorf("unexpected default OAuth HideInternalAuth setting, want:, got: %s", updatedSettings.OAuthSettings.LogoutURI) } diff --git a/api/datastore/migrate_dbversion33_test.go b/api/datastore/migrate_dbversion33_test.go index c789b5042..df964d718 100644 --- a/api/datastore/migrate_dbversion33_test.go +++ b/api/datastore/migrate_dbversion33_test.go @@ -10,8 +10,7 @@ import ( ) func TestMigrateStackEntryPoint(t *testing.T) { - _, store, teardown := MustNewTestStore(t, false, true) - defer teardown() + _, store := MustNewTestStore(t, false, true) stackService := store.Stack() diff --git a/api/datastore/teststore.go b/api/datastore/teststore.go index c050537ee..5df7dccc3 100644 --- a/api/datastore/teststore.go +++ b/api/datastore/teststore.go @@ -15,13 +15,15 @@ func (store *Store) GetConnection() portainer.Connection { return store.connection } -func MustNewTestStore(t testing.TB, init, secure bool) (bool, *Store, func()) { +func MustNewTestStore(t testing.TB, init, secure bool) (bool, *Store) { newStore, store, teardown, err := NewTestStore(t, init, secure) if err != nil { log.Fatal().Err(err).Msg("") } - return newStore, store, teardown + t.Cleanup(teardown) + + return newStore, store } func NewTestStore(t testing.TB, init, secure bool) (bool, *Store, func(), error) { diff --git a/api/http/handler/customtemplates/customtemplate_git_fetch_test.go b/api/http/handler/customtemplates/customtemplate_git_fetch_test.go index 53e48720a..6dfda8491 100644 --- a/api/http/handler/customtemplates/customtemplate_git_fetch_test.go +++ b/api/http/handler/customtemplates/customtemplate_git_fetch_test.go @@ -91,8 +91,7 @@ func singleAPIRequest(h *Handler, jwt string, is *assert.Assertions, expect stri func Test_customTemplateGitFetch(t *testing.T) { is := assert.New(t) - _, store, teardown := datastore.MustNewTestStore(t, true, true) - defer teardown() + _, store := datastore.MustNewTestStore(t, true, true) // create user(s) user1 := &portainer.User{ID: 1, Username: "user-1", Role: portainer.StandardUserRole, PortainerAuthorizations: authorization.DefaultPortainerAuthorizations()} diff --git a/api/http/handler/edgestacks/edgestack_create_test.go b/api/http/handler/edgestacks/edgestack_create_test.go index 26ae247a3..2a8910982 100644 --- a/api/http/handler/edgestacks/edgestack_create_test.go +++ b/api/http/handler/edgestacks/edgestack_create_test.go @@ -13,8 +13,7 @@ import ( // Create func TestCreateAndInspect(t *testing.T) { - handler, rawAPIKey, teardown := setupHandler(t) - defer teardown() + handler, rawAPIKey := setupHandler(t) // Create Endpoint, EdgeGroup and EndpointRelation endpoint := createEndpoint(t, handler.DataStore) @@ -100,8 +99,7 @@ func TestCreateAndInspect(t *testing.T) { } func TestCreateWithInvalidPayload(t *testing.T) { - handler, rawAPIKey, teardown := setupHandler(t) - defer teardown() + handler, rawAPIKey := setupHandler(t) endpoint := createEndpoint(t, handler.DataStore) edgeStack := createEdgeStack(t, handler.DataStore, endpoint.ID) diff --git a/api/http/handler/edgestacks/edgestack_delete_test.go b/api/http/handler/edgestacks/edgestack_delete_test.go index dacf94898..a225ff468 100644 --- a/api/http/handler/edgestacks/edgestack_delete_test.go +++ b/api/http/handler/edgestacks/edgestack_delete_test.go @@ -12,8 +12,7 @@ import ( // Delete func TestDeleteAndInspect(t *testing.T) { - handler, rawAPIKey, teardown := setupHandler(t) - defer teardown() + handler, rawAPIKey := setupHandler(t) // Create endpoint := createEndpoint(t, handler.DataStore) @@ -73,8 +72,7 @@ func TestDeleteAndInspect(t *testing.T) { } func TestDeleteInvalidEdgeStack(t *testing.T) { - handler, rawAPIKey, teardown := setupHandler(t) - defer teardown() + handler, rawAPIKey := setupHandler(t) cases := []struct { Name string diff --git a/api/http/handler/edgestacks/edgestack_inspect_test.go b/api/http/handler/edgestacks/edgestack_inspect_test.go index f76797a99..c73f536fa 100644 --- a/api/http/handler/edgestacks/edgestack_inspect_test.go +++ b/api/http/handler/edgestacks/edgestack_inspect_test.go @@ -8,8 +8,7 @@ import ( // Inspect func TestInspectInvalidEdgeID(t *testing.T) { - handler, rawAPIKey, teardown := setupHandler(t) - defer teardown() + handler, rawAPIKey := setupHandler(t) cases := []struct { Name string diff --git a/api/http/handler/edgestacks/edgestack_status_delete_test.go b/api/http/handler/edgestacks/edgestack_status_delete_test.go index 0487282e5..7a3db1315 100644 --- a/api/http/handler/edgestacks/edgestack_status_delete_test.go +++ b/api/http/handler/edgestacks/edgestack_status_delete_test.go @@ -10,8 +10,7 @@ import ( ) func TestDeleteStatus(t *testing.T) { - handler, _, teardown := setupHandler(t) - defer teardown() + handler, _ := setupHandler(t) endpoint := createEndpoint(t, handler.DataStore) edgeStack := createEdgeStack(t, handler.DataStore, endpoint.ID) diff --git a/api/http/handler/edgestacks/edgestack_status_update_test.go b/api/http/handler/edgestacks/edgestack_status_update_test.go index 77d844e20..0e1f72a99 100644 --- a/api/http/handler/edgestacks/edgestack_status_update_test.go +++ b/api/http/handler/edgestacks/edgestack_status_update_test.go @@ -13,8 +13,7 @@ import ( // Update Status func TestUpdateStatusAndInspect(t *testing.T) { - handler, rawAPIKey, teardown := setupHandler(t) - defer teardown() + handler, rawAPIKey := setupHandler(t) endpoint := createEndpoint(t, handler.DataStore) edgeStack := createEdgeStack(t, handler.DataStore, endpoint.ID) @@ -79,8 +78,7 @@ func TestUpdateStatusAndInspect(t *testing.T) { } } func TestUpdateStatusWithInvalidPayload(t *testing.T) { - handler, _, teardown := setupHandler(t) - defer teardown() + handler, _ := setupHandler(t) endpoint := createEndpoint(t, handler.DataStore) edgeStack := createEdgeStack(t, handler.DataStore, endpoint.ID) diff --git a/api/http/handler/edgestacks/edgestack_test.go b/api/http/handler/edgestacks/edgestack_test.go index 28c6de708..75e25b18b 100644 --- a/api/http/handler/edgestacks/edgestack_test.go +++ b/api/http/handler/edgestacks/edgestack_test.go @@ -19,28 +19,25 @@ import ( ) // Helpers -func setupHandler(t *testing.T) (*Handler, string, func()) { +func setupHandler(t *testing.T) (*Handler, string) { t.Helper() - _, store, storeTeardown := datastore.MustNewTestStore(t, true, true) + _, store := datastore.MustNewTestStore(t, true, true) jwtService, err := jwt.NewService("1h", store) if err != nil { - storeTeardown() t.Fatal(err) } user := &portainer.User{ID: 2, Username: "admin", Role: portainer.AdministratorRole} err = store.User().Create(user) if err != nil { - storeTeardown() t.Fatal(err) } apiKeyService := apikey.NewAPIKeyService(store.APIKeyRepository(), store.User()) rawAPIKey, _, err := apiKeyService.GenerateApiKey(*user, "test") if err != nil { - storeTeardown() t.Fatal(err) } @@ -56,7 +53,6 @@ func setupHandler(t *testing.T) (*Handler, string, func()) { fs, err := filesystem.NewService(tmpDir, "") if err != nil { - storeTeardown() t.Fatal(err) } handler.FileService = fs @@ -74,7 +70,7 @@ func setupHandler(t *testing.T) (*Handler, string, func()) { handler.GitService = testhelpers.NewGitService(errors.New("Clone error"), "git-service-id") - return handler, rawAPIKey, storeTeardown + return handler, rawAPIKey } func createEndpointWithId(t *testing.T, store dataservices.DataStore, endpointID portainer.EndpointID) portainer.Endpoint { diff --git a/api/http/handler/edgestacks/edgestack_update_test.go b/api/http/handler/edgestacks/edgestack_update_test.go index 874ee1448..deccb3690 100644 --- a/api/http/handler/edgestacks/edgestack_update_test.go +++ b/api/http/handler/edgestacks/edgestack_update_test.go @@ -14,8 +14,7 @@ import ( // Update func TestUpdateAndInspect(t *testing.T) { - handler, rawAPIKey, teardown := setupHandler(t) - defer teardown() + handler, rawAPIKey := setupHandler(t) endpoint := createEndpoint(t, handler.DataStore) edgeStack := createEdgeStack(t, handler.DataStore, endpoint.ID) @@ -116,8 +115,7 @@ func TestUpdateAndInspect(t *testing.T) { } func TestUpdateWithInvalidEdgeGroups(t *testing.T) { - handler, rawAPIKey, teardown := setupHandler(t) - defer teardown() + handler, rawAPIKey := setupHandler(t) endpoint := createEndpoint(t, handler.DataStore) edgeStack := createEdgeStack(t, handler.DataStore, endpoint.ID) @@ -197,8 +195,7 @@ func TestUpdateWithInvalidEdgeGroups(t *testing.T) { } func TestUpdateWithInvalidPayload(t *testing.T) { - handler, rawAPIKey, teardown := setupHandler(t) - defer teardown() + handler, rawAPIKey := setupHandler(t) endpoint := createEndpoint(t, handler.DataStore) edgeStack := createEdgeStack(t, handler.DataStore, endpoint.ID) diff --git a/api/http/handler/endpointedge/endpointedge_status_inspect_test.go b/api/http/handler/endpointedge/endpointedge_status_inspect_test.go index 91c32a0f6..f43b8a7a1 100644 --- a/api/http/handler/endpointedge/endpointedge_status_inspect_test.go +++ b/api/http/handler/endpointedge/endpointedge_status_inspect_test.go @@ -73,42 +73,35 @@ var endpointTestCases = []endpointTestCase{ }, } -func setupHandler(t *testing.T) (*Handler, func(), error) { +func mustSetupHandler(t *testing.T) *Handler { tmpDir := t.TempDir() fs, err := filesystem.NewService(tmpDir, "") if err != nil { - return nil, nil, fmt.Errorf("could not start a new filesystem service: %w", err) + t.Fatalf("could not start a new filesystem service: %s", err) } - _, store, storeTeardown := datastore.MustNewTestStore(t, true, true) + _, store := datastore.MustNewTestStore(t, true, true) ctx := context.Background() shutdownCtx, cancelFn := context.WithCancel(ctx) - - teardown := func() { - cancelFn() - storeTeardown() - } + t.Cleanup(cancelFn) jwtService, err := jwt.NewService("1h", store) if err != nil { - teardown() - return nil, nil, fmt.Errorf("could not start a new jwt service: %w", err) + t.Fatalf("could not start a new JWT service: %s", err) } apiKeyService := apikey.NewAPIKeyService(nil, nil) settings, err := store.Settings().Settings() if err != nil { - teardown() - return nil, nil, fmt.Errorf("could not create new settings: %w", err) + t.Fatalf("could not create new settings: %s", err) } settings.TrustOnFirstConnect = true err = store.Settings().UpdateSettings(settings) if err != nil { - teardown() - return nil, nil, fmt.Errorf("could not update settings: %w", err) + t.Fatalf("could not update settings: %s", err) } handler := NewHandler( @@ -120,7 +113,7 @@ func setupHandler(t *testing.T) (*Handler, func(), error) { handler.ReverseTunnelService = chisel.NewService(store, shutdownCtx) - return handler, teardown, nil + return handler } func createEndpoint(handler *Handler, endpoint portainer.Endpoint, endpointRelation portainer.EndpointRelation) (err error) { @@ -138,22 +131,16 @@ func createEndpoint(handler *Handler, endpoint portainer.Endpoint, endpointRelat } func TestMissingEdgeIdentifier(t *testing.T) { - handler, teardown, err := setupHandler(t) - defer teardown() - - if err != nil { - t.Fatal(err) - } - + handler := mustSetupHandler(t) endpointID := portainer.EndpointID(45) - err = createEndpoint(handler, portainer.Endpoint{ + + err := createEndpoint(handler, portainer.Endpoint{ ID: endpointID, Name: "endpoint-id-45", Type: portainer.EdgeAgentOnDockerEnvironment, URL: "https://portainer.io:9443", EdgeID: "edge-id", }, portainer.EndpointRelation{EndpointID: endpointID}) - if err != nil { t.Fatal(err) } @@ -172,15 +159,10 @@ func TestMissingEdgeIdentifier(t *testing.T) { } func TestWithEndpoints(t *testing.T) { - handler, teardown, err := setupHandler(t) - defer teardown() - - if err != nil { - t.Fatal(err) - } + handler := mustSetupHandler(t) for _, test := range endpointTestCases { - err = createEndpoint(handler, test.endpoint, test.endpointRelation) + err := createEndpoint(handler, test.endpoint, test.endpointRelation) if err != nil { t.Fatal(err) } @@ -203,12 +185,7 @@ func TestWithEndpoints(t *testing.T) { } func TestLastCheckInDateIncreases(t *testing.T) { - handler, teardown, err := setupHandler(t) - defer teardown() - - if err != nil { - t.Fatal(err) - } + handler := mustSetupHandler(t) endpointID := portainer.EndpointID(56) endpoint := portainer.Endpoint{ @@ -224,7 +201,7 @@ func TestLastCheckInDateIncreases(t *testing.T) { EndpointID: endpoint.ID, } - err = createEndpoint(handler, endpoint, endpointRelation) + err := createEndpoint(handler, endpoint, endpointRelation) if err != nil { t.Fatal(err) } @@ -254,12 +231,7 @@ func TestLastCheckInDateIncreases(t *testing.T) { } func TestEmptyEdgeIdWithAgentPlatformHeader(t *testing.T) { - handler, teardown, err := setupHandler(t) - defer teardown() - - if err != nil { - t.Fatal(err) - } + handler := mustSetupHandler(t) endpointID := portainer.EndpointID(44) edgeId := "edge-id" @@ -274,7 +246,7 @@ func TestEmptyEdgeIdWithAgentPlatformHeader(t *testing.T) { EndpointID: endpoint.ID, } - err = createEndpoint(handler, endpoint, endpointRelation) + err := createEndpoint(handler, endpoint, endpointRelation) if err != nil { t.Fatal(err) } @@ -302,12 +274,7 @@ func TestEmptyEdgeIdWithAgentPlatformHeader(t *testing.T) { } func TestEdgeStackStatus(t *testing.T) { - handler, teardown, err := setupHandler(t) - defer teardown() - - if err != nil { - t.Fatal(err) - } + handler := mustSetupHandler(t) endpointID := portainer.EndpointID(7) endpoint := portainer.Endpoint{ @@ -343,7 +310,7 @@ func TestEdgeStackStatus(t *testing.T) { } handler.DataStore.EdgeStack().Create(edgeStack.ID, &edgeStack) - err = createEndpoint(handler, endpoint, endpointRelation) + err := createEndpoint(handler, endpoint, endpointRelation) if err != nil { t.Fatal(err) } @@ -374,12 +341,7 @@ func TestEdgeStackStatus(t *testing.T) { } func TestEdgeJobsResponse(t *testing.T) { - handler, teardown, err := setupHandler(t) - defer teardown() - - if err != nil { - t.Fatal(err) - } + handler := mustSetupHandler(t) endpointID := portainer.EndpointID(77) endpoint := portainer.Endpoint{ @@ -395,7 +357,7 @@ func TestEdgeJobsResponse(t *testing.T) { EndpointID: endpoint.ID, } - err = createEndpoint(handler, endpoint, endpointRelation) + err := createEndpoint(handler, endpoint, endpointRelation) if err != nil { t.Fatal(err) } diff --git a/api/http/handler/endpoints/endpoint_delete_test.go b/api/http/handler/endpoints/endpoint_delete_test.go index 7865a19c0..93052c371 100644 --- a/api/http/handler/endpoints/endpoint_delete_test.go +++ b/api/http/handler/endpoints/endpoint_delete_test.go @@ -19,8 +19,7 @@ import ( func TestEndpointDeleteEdgeGroupsConcurrently(t *testing.T) { const endpointsCount = 100 - _, store, teardown := datastore.MustNewTestStore(t, true, false) - defer teardown() + _, store := datastore.MustNewTestStore(t, true, false) user := &portainer.User{ID: 2, Username: "admin", Role: portainer.AdministratorRole} err := store.User().Create(user) diff --git a/api/http/handler/endpoints/endpoint_list_test.go b/api/http/handler/endpoints/endpoint_list_test.go index 8bbefbf28..8ae1ba3bd 100644 --- a/api/http/handler/endpoints/endpoint_list_test.go +++ b/api/http/handler/endpoints/endpoint_list_test.go @@ -22,7 +22,6 @@ type endpointListTest struct { } func Test_EndpointList_AgentVersion(t *testing.T) { - version1Endpoint := portainer.Endpoint{ ID: 1, GroupID: 1, @@ -39,15 +38,13 @@ func Test_EndpointList_AgentVersion(t *testing.T) { noVersionEndpoint := portainer.Endpoint{ID: 3, Type: portainer.AgentOnDockerEnvironment, GroupID: 1} notAgentEnvironments := portainer.Endpoint{ID: 4, Type: portainer.DockerEnvironment, GroupID: 1} - handler, teardown := setupEndpointListHandler(t, []portainer.Endpoint{ + handler := setupEndpointListHandler(t, []portainer.Endpoint{ notAgentEnvironments, version1Endpoint, version2Endpoint, noVersionEndpoint, }) - defer teardown() - type endpointListAgentVersionTest struct { endpointListTest filter []string @@ -111,7 +108,7 @@ func Test_endpointList_edgeFilter(t *testing.T) { regularTrustedEdgeStandard := portainer.Endpoint{ID: 4, UserTrusted: true, Edge: portainer.EnvironmentEdgeSettings{AsyncMode: false}, GroupID: 1, Type: portainer.EdgeAgentOnDockerEnvironment} regularEndpoint := portainer.Endpoint{ID: 5, GroupID: 1, Type: portainer.DockerEnvironment} - handler, teardown := setupEndpointListHandler(t, []portainer.Endpoint{ + handler := setupEndpointListHandler(t, []portainer.Endpoint{ trustedEdgeAsync, untrustedEdgeAsync, regularUntrustedEdgeStandard, @@ -119,8 +116,6 @@ func Test_endpointList_edgeFilter(t *testing.T) { regularEndpoint, }) - defer teardown() - type endpointListEdgeTest struct { endpointListTest edgeAsync *bool @@ -184,9 +179,9 @@ func Test_endpointList_edgeFilter(t *testing.T) { } } -func setupEndpointListHandler(t *testing.T, endpoints []portainer.Endpoint) (handler *Handler, teardown func()) { +func setupEndpointListHandler(t *testing.T, endpoints []portainer.Endpoint) *Handler { is := assert.New(t) - _, store, teardown := datastore.MustNewTestStore(t, true, true) + _, store := datastore.MustNewTestStore(t, true, true) for _, endpoint := range endpoints { err := store.Endpoint().Create(&endpoint) @@ -197,13 +192,13 @@ func setupEndpointListHandler(t *testing.T, endpoints []portainer.Endpoint) (han is.NoError(err, "error creating a user") bouncer := testhelpers.NewTestRequestBouncer() - handler = NewHandler(bouncer, nil) + + handler := NewHandler(bouncer, nil) handler.DataStore = store handler.ComposeStackManager = testhelpers.NewComposeStackManager() - handler.SnapshotService, _ = snapshot.NewService("1s", store, nil, nil, nil) - return handler, teardown + return handler } func buildEndpointListRequest(query string) *http.Request { diff --git a/api/http/handler/endpoints/filter_test.go b/api/http/handler/endpoints/filter_test.go index dd90f9b65..e54af0aea 100644 --- a/api/http/handler/endpoints/filter_test.go +++ b/api/http/handler/endpoints/filter_test.go @@ -39,9 +39,7 @@ func Test_Filter_AgentVersion(t *testing.T) { notAgentEnvironments, } - handler, teardown := setupFilterTest(t, endpoints) - - defer teardown() + handler := setupFilterTest(t, endpoints) tests := []filterTest{ { @@ -89,9 +87,7 @@ func Test_Filter_edgeFilter(t *testing.T) { regularEndpoint, } - handler, teardown := setupFilterTest(t, endpoints) - - defer teardown() + handler := setupFilterTest(t, endpoints) tests := []filterTest{ { @@ -155,9 +151,9 @@ func runTest(t *testing.T, test filterTest, handler *Handler, endpoints []portai } -func setupFilterTest(t *testing.T, endpoints []portainer.Endpoint) (handler *Handler, teardown func()) { +func setupFilterTest(t *testing.T, endpoints []portainer.Endpoint) *Handler { is := assert.New(t) - _, store, teardown := datastore.MustNewTestStore(t, true, true) + _, store := datastore.MustNewTestStore(t, true, true) for _, endpoint := range endpoints { err := store.Endpoint().Create(&endpoint) @@ -168,9 +164,9 @@ func setupFilterTest(t *testing.T, endpoints []portainer.Endpoint) (handler *Han is.NoError(err, "error creating a user") bouncer := testhelpers.NewTestRequestBouncer() - handler = NewHandler(bouncer, nil) + handler := NewHandler(bouncer, nil) handler.DataStore = store handler.ComposeStackManager = testhelpers.NewComposeStackManager() - return handler, teardown + return handler } diff --git a/api/http/handler/endpoints/utils_update_edge_groups_test.go b/api/http/handler/endpoints/utils_update_edge_groups_test.go index 90f036ac2..7d4967797 100644 --- a/api/http/handler/endpoints/utils_update_edge_groups_test.go +++ b/api/http/handler/endpoints/utils_update_edge_groups_test.go @@ -70,10 +70,8 @@ func Test_updateEdgeGroups(t *testing.T) { } testFn := func(t *testing.T, testCase testCase) { - is := assert.New(t) - _, store, teardown := datastore.MustNewTestStore(t, true, true) - defer teardown() + _, store := datastore.MustNewTestStore(t, true, true) err := store.Endpoint().Create(testCase.endpoint) is.NoError(err) diff --git a/api/http/handler/endpoints/utils_update_tags_test.go b/api/http/handler/endpoints/utils_update_tags_test.go index 501a40309..d36cc2aae 100644 --- a/api/http/handler/endpoints/utils_update_tags_test.go +++ b/api/http/handler/endpoints/utils_update_tags_test.go @@ -74,10 +74,8 @@ func Test_updateTags(t *testing.T) { } testFn := func(t *testing.T, testCase testCase) { - is := assert.New(t) - _, store, teardown := datastore.MustNewTestStore(t, true, true) - defer teardown() + _, store := datastore.MustNewTestStore(t, true, true) err := store.Endpoint().Create(testCase.endpoint) is.NoError(err) diff --git a/api/http/handler/helm/helm_delete_test.go b/api/http/handler/helm/helm_delete_test.go index d4aae9003..bece6fbde 100644 --- a/api/http/handler/helm/helm_delete_test.go +++ b/api/http/handler/helm/helm_delete_test.go @@ -22,8 +22,7 @@ import ( func Test_helmDelete(t *testing.T) { is := assert.New(t) - _, store, teardown := datastore.MustNewTestStore(t, true, true) - defer teardown() + _, store := datastore.MustNewTestStore(t, true, true) err := store.Endpoint().Create(&portainer.Endpoint{ID: 1}) is.NoError(err, "Error creating environment") diff --git a/api/http/handler/helm/helm_install_test.go b/api/http/handler/helm/helm_install_test.go index 0cc59eec5..6b061a16c 100644 --- a/api/http/handler/helm/helm_install_test.go +++ b/api/http/handler/helm/helm_install_test.go @@ -25,8 +25,7 @@ import ( func Test_helmInstall(t *testing.T) { is := assert.New(t) - _, store, teardown := datastore.MustNewTestStore(t, true, true) - defer teardown() + _, store := datastore.MustNewTestStore(t, true, true) err := store.Endpoint().Create(&portainer.Endpoint{ID: 1}) is.NoError(err, "error creating environment") diff --git a/api/http/handler/helm/helm_list_test.go b/api/http/handler/helm/helm_list_test.go index 00dac414e..d05ba7a73 100644 --- a/api/http/handler/helm/helm_list_test.go +++ b/api/http/handler/helm/helm_list_test.go @@ -24,8 +24,7 @@ import ( func Test_helmList(t *testing.T) { is := assert.New(t) - _, store, teardown := datastore.MustNewTestStore(t, true, true) - defer teardown() + _, store := datastore.MustNewTestStore(t, true, true) err := store.Endpoint().Create(&portainer.Endpoint{ID: 1}) assert.NoError(t, err, "error creating environment") diff --git a/api/http/handler/stacks/webhook_invoke_test.go b/api/http/handler/stacks/webhook_invoke_test.go index b81d2f395..7f50a2195 100644 --- a/api/http/handler/stacks/webhook_invoke_test.go +++ b/api/http/handler/stacks/webhook_invoke_test.go @@ -13,8 +13,7 @@ import ( ) func TestHandler_webhookInvoke(t *testing.T) { - _, store, teardown := datastore.MustNewTestStore(t, true, true) - defer teardown() + _, store := datastore.MustNewTestStore(t, true, true) webhookID := newGuidString(t) store.StackService.Create(&portainer.Stack{ diff --git a/api/http/handler/tags/tag_delete_test.go b/api/http/handler/tags/tag_delete_test.go index ccb283184..0c2c45738 100644 --- a/api/http/handler/tags/tag_delete_test.go +++ b/api/http/handler/tags/tag_delete_test.go @@ -17,8 +17,7 @@ import ( func TestTagDeleteEdgeGroupsConcurrently(t *testing.T) { const tagsCount = 100 - _, store, teardown := datastore.MustNewTestStore(t, true, false) - defer teardown() + _, store := datastore.MustNewTestStore(t, true, false) user := &portainer.User{ID: 2, Username: "admin", Role: portainer.AdministratorRole} err := store.User().Create(user) diff --git a/api/http/handler/teams/team_list_test.go b/api/http/handler/teams/team_list_test.go index 6d088f7a5..063aca097 100644 --- a/api/http/handler/teams/team_list_test.go +++ b/api/http/handler/teams/team_list_test.go @@ -21,8 +21,7 @@ import ( func Test_teamList(t *testing.T) { is := assert.New(t) - _, store, teardown := datastore.MustNewTestStore(t, true, true) - defer teardown() + _, store := datastore.MustNewTestStore(t, true, true) // create admin adminUser := &portainer.User{ID: 1, Username: "admin", Role: portainer.AdministratorRole} diff --git a/api/http/handler/users/user_create_access_token_test.go b/api/http/handler/users/user_create_access_token_test.go index 3150176a5..9d33f204c 100644 --- a/api/http/handler/users/user_create_access_token_test.go +++ b/api/http/handler/users/user_create_access_token_test.go @@ -21,8 +21,7 @@ import ( func Test_userCreateAccessToken(t *testing.T) { is := assert.New(t) - _, store, teardown := datastore.MustNewTestStore(t, true, true) - defer teardown() + _, store := datastore.MustNewTestStore(t, true, true) // create admin and standard user(s) adminUser := &portainer.User{ID: 1, Username: "admin", Role: portainer.AdministratorRole} diff --git a/api/http/handler/users/user_delete_test.go b/api/http/handler/users/user_delete_test.go index 426ae8be0..aa3e84f3a 100644 --- a/api/http/handler/users/user_delete_test.go +++ b/api/http/handler/users/user_delete_test.go @@ -17,8 +17,7 @@ import ( func Test_deleteUserRemovesAccessTokens(t *testing.T) { is := assert.New(t) - _, store, teardown := datastore.MustNewTestStore(t, true, true) - defer teardown() + _, store := datastore.MustNewTestStore(t, true, true) // create standard user user := &portainer.User{ID: 2, Username: "standard", Role: portainer.StandardUserRole} diff --git a/api/http/handler/users/user_get_access_tokens_test.go b/api/http/handler/users/user_get_access_tokens_test.go index a02e97118..f07d98e6f 100644 --- a/api/http/handler/users/user_get_access_tokens_test.go +++ b/api/http/handler/users/user_get_access_tokens_test.go @@ -20,8 +20,7 @@ import ( func Test_userGetAccessTokens(t *testing.T) { is := assert.New(t) - _, store, teardown := datastore.MustNewTestStore(t, true, true) - defer teardown() + _, store := datastore.MustNewTestStore(t, true, true) // create admin and standard user(s) adminUser := &portainer.User{ID: 1, Username: "admin", Role: portainer.AdministratorRole} diff --git a/api/http/handler/users/user_list_test.go b/api/http/handler/users/user_list_test.go index 40d4c7cf7..094c59530 100644 --- a/api/http/handler/users/user_list_test.go +++ b/api/http/handler/users/user_list_test.go @@ -23,8 +23,7 @@ import ( func Test_userList(t *testing.T) { is := assert.New(t) - _, store, teardown := datastore.MustNewTestStore(t, true, true) - defer teardown() + _, store := datastore.MustNewTestStore(t, true, true) // create admin and standard user(s) adminUser := &portainer.User{ID: 1, Username: "admin", Role: portainer.AdministratorRole} diff --git a/api/http/handler/users/user_remove_access_token_test.go b/api/http/handler/users/user_remove_access_token_test.go index 605637dc3..bc41a0c91 100644 --- a/api/http/handler/users/user_remove_access_token_test.go +++ b/api/http/handler/users/user_remove_access_token_test.go @@ -18,8 +18,7 @@ import ( func Test_userRemoveAccessToken(t *testing.T) { is := assert.New(t) - _, store, teardown := datastore.MustNewTestStore(t, true, true) - defer teardown() + _, store := datastore.MustNewTestStore(t, true, true) // create admin and standard user(s) adminUser := &portainer.User{ID: 1, Username: "admin", Role: portainer.AdministratorRole} diff --git a/api/http/handler/users/user_update_test.go b/api/http/handler/users/user_update_test.go index 32577da79..dc75fbc99 100644 --- a/api/http/handler/users/user_update_test.go +++ b/api/http/handler/users/user_update_test.go @@ -17,8 +17,7 @@ import ( func Test_updateUserRemovesAccessTokens(t *testing.T) { is := assert.New(t) - _, store, teardown := datastore.MustNewTestStore(t, true, true) - defer teardown() + _, store := datastore.MustNewTestStore(t, true, true) // create standard user user := &portainer.User{ID: 2, Username: "standard", Role: portainer.StandardUserRole} diff --git a/api/http/security/bouncer_test.go b/api/http/security/bouncer_test.go index 23c1170b0..4b93d23f6 100644 --- a/api/http/security/bouncer_test.go +++ b/api/http/security/bouncer_test.go @@ -37,8 +37,7 @@ func tokenLookupFail(r *http.Request) *portainer.TokenData { func Test_mwAuthenticateFirst(t *testing.T) { is := assert.New(t) - _, store, teardown := datastore.MustNewTestStore(t, true, true) - defer teardown() + _, store := datastore.MustNewTestStore(t, true, true) jwtService, err := jwt.NewService("1h", store) assert.NoError(t, err, "failed to create a copy of service") @@ -260,8 +259,7 @@ func Test_extractAPIKeyQueryParam(t *testing.T) { func Test_apiKeyLookup(t *testing.T) { is := assert.New(t) - _, store, teardown := datastore.MustNewTestStore(t, true, true) - defer teardown() + _, store := datastore.MustNewTestStore(t, true, true) // create standard user user := &portainer.User{ID: 2, Username: "standard", Role: portainer.StandardUserRole} diff --git a/api/stacks/deployments/deploy_test.go b/api/stacks/deployments/deploy_test.go index 146c50b4c..aec88ebf3 100644 --- a/api/stacks/deployments/deploy_test.go +++ b/api/stacks/deployments/deploy_test.go @@ -5,11 +5,11 @@ import ( "strings" "testing" + portainer "github.com/portainer/portainer/api" "github.com/portainer/portainer/api/datastore" + gittypes "github.com/portainer/portainer/api/git/types" "github.com/portainer/portainer/api/internal/testhelpers" - portainer "github.com/portainer/portainer/api" - gittypes "github.com/portainer/portainer/api/git/types" "github.com/stretchr/testify/assert" ) @@ -55,8 +55,7 @@ func (s *noopDeployer) StopRemoteSwarmStack(stack *portainer.Stack, endpoint *po } func Test_redeployWhenChanged_FailsWhenCannotFindStack(t *testing.T) { - _, store, teardown := datastore.MustNewTestStore(t, true, true) - defer teardown() + _, store := datastore.MustNewTestStore(t, true, true) err := RedeployWhenChanged(1, nil, store, nil) assert.Error(t, err) @@ -64,8 +63,7 @@ func Test_redeployWhenChanged_FailsWhenCannotFindStack(t *testing.T) { } func Test_redeployWhenChanged_DoesNothingWhenNotAGitBasedStack(t *testing.T) { - _, store, teardown := datastore.MustNewTestStore(t, true, true) - defer teardown() + _, store := datastore.MustNewTestStore(t, true, true) admin := &portainer.User{ID: 1, Username: "admin"} err := store.User().Create(admin) @@ -79,8 +77,7 @@ func Test_redeployWhenChanged_DoesNothingWhenNotAGitBasedStack(t *testing.T) { } func Test_redeployWhenChanged_DoesNothingWhenNoGitChanges(t *testing.T) { - _, store, teardown := datastore.MustNewTestStore(t, true, true) - defer teardown() + _, store := datastore.MustNewTestStore(t, true, true) tmpDir := t.TempDir() @@ -110,8 +107,7 @@ func Test_redeployWhenChanged_DoesNothingWhenNoGitChanges(t *testing.T) { func Test_redeployWhenChanged_FailsWhenCannotClone(t *testing.T) { cloneErr := errors.New("failed to clone") - _, store, teardown := datastore.MustNewTestStore(t, true, true) - defer teardown() + _, store := datastore.MustNewTestStore(t, true, true) admin := &portainer.User{ID: 1, Username: "admin"} err := store.User().Create(admin) @@ -138,8 +134,7 @@ func Test_redeployWhenChanged_FailsWhenCannotClone(t *testing.T) { } func Test_redeployWhenChanged(t *testing.T) { - _, store, teardown := datastore.MustNewTestStore(t, true, true) - defer teardown() + _, store := datastore.MustNewTestStore(t, true, true) tmpDir := t.TempDir() @@ -191,8 +186,7 @@ func Test_redeployWhenChanged(t *testing.T) { } func Test_getUserRegistries(t *testing.T) { - _, store, teardown := datastore.MustNewTestStore(t, true, true) - defer teardown() + _, store := datastore.MustNewTestStore(t, true, true) endpointID := 123