From e75979440812832ae94bde9f37bd2047a1e388f0 Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Sun, 9 Jun 2024 15:49:22 +0200 Subject: [PATCH] tests(cmd): add coverage for migrateActionsArtifacts Also convert a comment into a warning in the logs when the deletion of an artifact cannot find the file in the destination storage. The case were an error happens while deleting the file is not covered as it would require to mock the storage.Copy function. --- cmd/migrate_storage.go | 2 +- cmd/migrate_storage_test.go | 75 +++++++++++++++++++++++++++++++++---- 2 files changed, 68 insertions(+), 9 deletions(-) diff --git a/cmd/migrate_storage.go b/cmd/migrate_storage.go index 69fdb90f33..3a69b555e0 100644 --- a/cmd/migrate_storage.go +++ b/cmd/migrate_storage.go @@ -170,8 +170,8 @@ func migrateActionsArtifacts(ctx context.Context, dstStorage storage.ObjectStora _, err := storage.Copy(dstStorage, artifact.StoragePath, storage.ActionsArtifacts, artifact.StoragePath) if err != nil { - // ignore files that do not exist if errors.Is(err, fs.ErrNotExist) { + log.Warn("ignored: actions artifact %s exists in the database but not in storage", artifact.StoragePath) return nil } return err diff --git a/cmd/migrate_storage_test.go b/cmd/migrate_storage_test.go index 5d8c867993..fa7067f97b 100644 --- a/cmd/migrate_storage_test.go +++ b/cmd/migrate_storage_test.go @@ -5,10 +5,12 @@ package cmd import ( "context" + "io" "os" "strings" "testing" + "code.gitea.io/gitea/models/actions" "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/packages" "code.gitea.io/gitea/models/unittest" @@ -16,11 +18,28 @@ import ( packages_module "code.gitea.io/gitea/modules/packages" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/storage" + "code.gitea.io/gitea/modules/test" packages_service "code.gitea.io/gitea/services/packages" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) +func createLocalStorage(t *testing.T) (storage.ObjectStorage, string) { + t.Helper() + + p := t.TempDir() + + storage, err := storage.NewLocalStorage( + context.Background(), + &setting.Storage{ + Path: p, + }) + require.NoError(t, err) + + return storage, p +} + func TestMigratePackages(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) @@ -55,14 +74,7 @@ func TestMigratePackages(t *testing.T) { ctx := context.Background() - p := t.TempDir() - - dstStorage, err := storage.NewLocalStorage( - ctx, - &setting.Storage{ - Path: p, - }) - assert.NoError(t, err) + dstStorage, p := createLocalStorage(t) err = migratePackages(ctx, dstStorage) assert.NoError(t, err) @@ -73,3 +85,50 @@ func TestMigratePackages(t *testing.T) { assert.EqualValues(t, "01", entries[0].Name()) assert.EqualValues(t, "tmp", entries[1].Name()) } + +func TestMigrateActionsArtifacts(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + srcStorage, _ := createLocalStorage(t) + defer test.MockVariableValue(&storage.ActionsArtifacts, srcStorage)() + id := int64(0) + + addArtifact := func(storagePath string, status actions.ArtifactStatus) { + id++ + artifact := &actions.ActionArtifact{ + ID: id, + ArtifactName: storagePath, + StoragePath: storagePath, + Status: int64(status), + } + _, err := db.GetEngine(db.DefaultContext).Insert(artifact) + require.NoError(t, err) + srcStorage.Save(storagePath, strings.NewReader(storagePath), -1) + } + + exists := "/exists" + addArtifact(exists, actions.ArtifactStatusUploadConfirmed) + + expired := "/expired" + addArtifact(expired, actions.ArtifactStatusExpired) + + notFound := "/notfound" + addArtifact(notFound, actions.ArtifactStatusUploadConfirmed) + srcStorage.Delete(notFound) + + dstStorage, _ := createLocalStorage(t) + + assert.NoError(t, migrateActionsArtifacts(db.DefaultContext, dstStorage)) + + object, err := dstStorage.Open(exists) + assert.NoError(t, err) + buf, err := io.ReadAll(object) + require.NoError(t, err) + assert.Equal(t, exists, string(buf)) + + _, err = dstStorage.Stat(expired) + assert.Error(t, err) + + _, err = dstStorage.Stat(notFound) + assert.Error(t, err) +}