From 902294b70da5513168fba159ea90739823ef6319 Mon Sep 17 00:00:00 2001 From: ashmrtn <3891298+ashmrtn@users.noreply.github.com> Date: Thu, 23 Jun 2022 11:55:26 -0700 Subject: [PATCH] S3 test prefix naming (#228) * Update how S3 storage structs are generated * fix bug in printing year of date * use the name of the test instead of trying to pull name from runtime * always log the time when making the storage struct * don't allow user to specify prefix * Fixup tests for new test storage API * Update function name and comment --- .../connector/graph_connector_test.go | 1 - src/internal/kopia/kopia_test.go | 46 +++++++++---------- src/internal/operations/backup_test.go | 4 +- src/internal/testing/integration_runners.go | 10 ++-- src/internal/testing/storage.go | 12 +++-- src/pkg/repository/repository_test.go | 27 ++++------- 6 files changed, 44 insertions(+), 56 deletions(-) diff --git a/src/internal/connector/graph_connector_test.go b/src/internal/connector/graph_connector_test.go index 6ce32d4b4..4942c22b6 100644 --- a/src/internal/connector/graph_connector_test.go +++ b/src/internal/connector/graph_connector_test.go @@ -43,7 +43,6 @@ func (suite *GraphConnectorIntegrationSuite) SetupSuite() { func (suite *GraphConnectorIntegrationSuite) TestGraphConnector() { ctesting.LogTimeOfTest(suite.T()) suite.NotNil(suite.connector) - } type DiconnectedGraphConnectorSuite struct { diff --git a/src/internal/kopia/kopia_test.go b/src/internal/kopia/kopia_test.go index ca6c16d95..8c6544a68 100644 --- a/src/internal/kopia/kopia_test.go +++ b/src/internal/kopia/kopia_test.go @@ -31,8 +31,8 @@ var ( testFileData = []byte("abcdefghijklmnopqrstuvwxyz") ) -func openKopiaRepo(ctx context.Context, prefix string) (*KopiaWrapper, error) { - storage, err := ctesting.NewS3Storage(prefix) +func openKopiaRepo(t *testing.T, ctx context.Context) (*KopiaWrapper, error) { + storage, err := ctesting.NewPrefixedS3Storage(t) if err != nil { return nil, err } @@ -253,23 +253,23 @@ func (suite *KopiaIntegrationSuite) SetupSuite() { func (suite *KopiaIntegrationSuite) TestCloseTwiceDoesNotCrash() { ctx := context.Background() - timeOfTest := ctesting.LogTimeOfTest(suite.T()) + t := suite.T() - k, err := openKopiaRepo(ctx, "init-s3-"+timeOfTest) - require.NoError(suite.T(), err) - assert.NoError(suite.T(), k.Close(ctx)) - assert.Nil(suite.T(), k.rep) - assert.NoError(suite.T(), k.Close(ctx)) + k, err := openKopiaRepo(t, ctx) + require.NoError(t, err) + assert.NoError(t, k.Close(ctx)) + assert.Nil(t, k.rep) + assert.NoError(t, k.Close(ctx)) } func (suite *KopiaIntegrationSuite) TestBackupCollections() { ctx := context.Background() - timeOfTest := ctesting.LogTimeOfTest(suite.T()) + t := suite.T() - k, err := openKopiaRepo(ctx, "init-s3-"+timeOfTest) - require.NoError(suite.T(), err) + k, err := openKopiaRepo(t, ctx) + require.NoError(t, err) defer func() { - assert.NoError(suite.T(), k.Close(ctx)) + assert.NoError(t, k.Close(ctx)) }() collections := []connector.DataCollection{ @@ -284,12 +284,12 @@ func (suite *KopiaIntegrationSuite) TestBackupCollections() { } stats, err := k.BackupCollections(ctx, collections) - assert.NoError(suite.T(), err) - assert.Equal(suite.T(), stats.TotalFileCount, 47) - assert.Equal(suite.T(), stats.TotalDirectoryCount, 5) - assert.Equal(suite.T(), stats.IgnoredErrorCount, 0) - assert.Equal(suite.T(), stats.ErrorCount, 0) - assert.False(suite.T(), stats.Incomplete) + assert.NoError(t, err) + assert.Equal(t, stats.TotalFileCount, 47) + assert.Equal(t, stats.TotalDirectoryCount, 5) + assert.Equal(t, stats.IgnoredErrorCount, 0) + assert.Equal(t, stats.ErrorCount, 0) + assert.False(t, stats.Incomplete) } func setupSimpleRepo(t *testing.T, ctx context.Context, k *KopiaWrapper) manifest.ID { @@ -316,10 +316,9 @@ func setupSimpleRepo(t *testing.T, ctx context.Context, k *KopiaWrapper) manifes func (suite *KopiaIntegrationSuite) TestBackupAndRestoreSingleItem() { ctx := context.Background() - timeOfTest := ctesting.LogTimeOfTest(suite.T()) t := suite.T() - k, err := openKopiaRepo(ctx, "backup-restore-single-item-"+timeOfTest) + k, err := openKopiaRepo(t, ctx) require.NoError(t, err) defer func() { assert.NoError(t, k.Close(ctx)) @@ -381,9 +380,9 @@ func (suite *KopiaIntegrationSuite) TestBackupAndRestoreSingleItem_Errors() { for _, test := range table { suite.T().Run(test.name, func(t *testing.T) { ctx := context.Background() - timeOfTest := ctesting.LogTimeOfTest(t) + ctesting.LogTimeOfTest(t) - k, err := openKopiaRepo(ctx, "backup-restore-single-item-error-"+test.name+"-"+timeOfTest) + k, err := openKopiaRepo(t, ctx) require.NoError(t, err) defer func() { assert.NoError(t, k.Close(ctx)) @@ -429,9 +428,8 @@ func (suite *KopiaIntegrationSuite) TestBackupAndRestoreSingleItem_Errors2() { for _, test := range table { suite.T().Run(test.name, func(t *testing.T) { ctx := context.Background() - timeOfTest := ctesting.LogTimeOfTest(t) - k, err := openKopiaRepo(ctx, "backup-restore-single-item-error2-"+test.name+"-"+timeOfTest) + k, err := openKopiaRepo(t, ctx) require.NoError(t, err) defer func() { assert.NoError(t, k.Close(ctx)) diff --git a/src/internal/operations/backup_test.go b/src/internal/operations/backup_test.go index 073144d70..4fbf757bd 100644 --- a/src/internal/operations/backup_test.go +++ b/src/internal/operations/backup_test.go @@ -66,8 +66,6 @@ func (suite *BackupOpIntegrationSuite) TestNewBackupOperation() { func (suite *BackupOpIntegrationSuite) TestBackup_Run() { t := suite.T() ctx := context.Background() - timeOfTest := ctesting.LogTimeOfTest(t) - prefix := "backup-op-run-" + timeOfTest // m365User := "lidiah@8qzvrj.onmicrosoft.com" // not the user we want to use, but all the others are @@ -81,7 +79,7 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run() { } // need to initialize the repository before we can test connecting to it. - st, err := ctesting.NewS3Storage(prefix) + st, err := ctesting.NewPrefixedS3Storage(t) require.NoError(t, err) r, err := repository.Initialize(ctx, acct, st) diff --git a/src/internal/testing/integration_runners.go b/src/internal/testing/integration_runners.go index 0711eb0ee..c7dc26006 100644 --- a/src/internal/testing/integration_runners.go +++ b/src/internal/testing/integration_runners.go @@ -3,7 +3,6 @@ package testing import ( "fmt" "os" - "runtime" "strings" "testing" "time" @@ -35,13 +34,12 @@ func RunOnAny(tests ...string) error { // LogTimeOfTest logs the test name and the time that it was run. func LogTimeOfTest(t *testing.T) string { - now := time.Now().UTC().Format("2016-01-02T15:04:05.0000") - pc, _, _, ok := runtime.Caller(1) - details := runtime.FuncForPC(pc) - if !ok || details != nil { + now := time.Now().UTC().Format("2006-01-02T15:04:05.0000") + name := t.Name() + if name == "" { t.Logf("Test run at %s.", now) return now } - t.Logf("%s() run at %s", details.Name(), now) + t.Logf("%s run at %s", name, now) return now } diff --git a/src/internal/testing/storage.go b/src/internal/testing/storage.go index 4730e85b3..3325703ac 100644 --- a/src/internal/testing/storage.go +++ b/src/internal/testing/storage.go @@ -1,6 +1,8 @@ package testing import ( + "testing" + "github.com/alcionai/corso/pkg/credentials" "github.com/alcionai/corso/pkg/storage" "github.com/pkg/errors" @@ -12,9 +14,11 @@ var AWSCredentialEnvs = []string{ credentials.AWSSessionToken, } -// NewS3Storage returns a storage.Storage object initialized with environment -// variables used for integration tests that use S3. -func NewS3Storage(prefix string) (storage.Storage, error) { +// NewPrefixedS3Storage returns a storage.Storage object initialized with environment +// variables used for integration tests that use S3. The prefix for the storage +// path will be unique. +func NewPrefixedS3Storage(t *testing.T) (storage.Storage, error) { + now := LogTimeOfTest(t) cfg, err := readTestConfig() if err != nil { return storage.Storage{}, errors.Wrap(err, "configuring storage from test file") @@ -25,7 +29,7 @@ func NewS3Storage(prefix string) (storage.Storage, error) { storage.S3Config{ AWS: credentials.GetAWS(nil), Bucket: cfg[testCfgBucket], - Prefix: prefix, + Prefix: t.Name() + "-" + now, }, storage.CommonConfig{ Corso: credentials.GetCorso(), diff --git a/src/pkg/repository/repository_test.go b/src/pkg/repository/repository_test.go index cbaaed993..4c2384e62 100644 --- a/src/pkg/repository/repository_test.go +++ b/src/pkg/repository/repository_test.go @@ -106,25 +106,22 @@ func (suite *RepositoryIntegrationSuite) SetupSuite() { func (suite *RepositoryIntegrationSuite) TestInitialize() { ctx := context.Background() - timeOfTest := ctesting.LogTimeOfTest(suite.T()) table := []struct { - prefix string + name string account repository.Account - storage func() (storage.Storage, error) + storage func(*testing.T) (storage.Storage, error) errCheck assert.ErrorAssertionFunc }{ { - prefix: "repository-init-s3-" + timeOfTest, - storage: func() (storage.Storage, error) { - return ctesting.NewS3Storage("repository-init-s3-" + timeOfTest) - }, + name: "success", + storage: ctesting.NewPrefixedS3Storage, errCheck: assert.NoError, }, } for _, test := range table { - suite.T().Run(test.prefix, func(t *testing.T) { - st, err := test.storage() + suite.T().Run(test.name, func(t *testing.T) { + st, err := test.storage(t) assert.NoError(t, err) r, err := repository.Initialize(ctx, test.account, st) if err == nil { @@ -141,11 +138,9 @@ func (suite *RepositoryIntegrationSuite) TestInitialize() { func (suite *RepositoryIntegrationSuite) TestConnect() { t := suite.T() ctx := context.Background() - timeOfTest := ctesting.LogTimeOfTest(t) - prefix := "repository-conn-s3-" + timeOfTest // need to initialize the repository before we can test connecting to it. - st, err := ctesting.NewS3Storage(prefix) + st, err := ctesting.NewPrefixedS3Storage(t) require.NoError(t, err) _, err = repository.Initialize(ctx, repository.Account{}, st) @@ -159,8 +154,6 @@ func (suite *RepositoryIntegrationSuite) TestConnect() { func (suite *RepositoryIntegrationSuite) TestNewBackup() { t := suite.T() ctx := context.Background() - timeOfTest := ctesting.LogTimeOfTest(t) - prefix := "repository-new-backup-" + timeOfTest m365 := credentials.GetM365() acct := repository.Account{ @@ -170,7 +163,7 @@ func (suite *RepositoryIntegrationSuite) TestNewBackup() { } // need to initialize the repository before we can test connecting to it. - st, err := ctesting.NewS3Storage(prefix) + st, err := ctesting.NewPrefixedS3Storage(t) require.NoError(t, err) r, err := repository.Initialize(ctx, acct, st) @@ -184,8 +177,6 @@ func (suite *RepositoryIntegrationSuite) TestNewBackup() { func (suite *RepositoryIntegrationSuite) TestNewRestore() { t := suite.T() ctx := context.Background() - timeOfTest := ctesting.LogTimeOfTest(t) - prefix := "repository-new-restore-" + timeOfTest m365 := credentials.GetM365() acct := repository.Account{ @@ -195,7 +186,7 @@ func (suite *RepositoryIntegrationSuite) TestNewRestore() { } // need to initialize the repository before we can test connecting to it. - st, err := ctesting.NewS3Storage(prefix) + st, err := ctesting.NewPrefixedS3Storage(t) require.NoError(t, err) r, err := repository.Initialize(ctx, acct, st)