diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index c10069382f2..3bd6959b99f 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -1308,8 +1308,8 @@ module Ci end def has_expired_test_reports? - strong_memoize(:artifacts_expired) do - !has_reports?(::Ci::JobArtifact.test_reports.not_expired) + strong_memoize(:has_expired_test_reports) do + has_reports?(::Ci::JobArtifact.test_reports.expired) end end diff --git a/app/models/concerns/ci/artifactable.rb b/app/models/concerns/ci/artifactable.rb index 27040a677ff..78340cf967b 100644 --- a/app/models/concerns/ci/artifactable.rb +++ b/app/models/concerns/ci/artifactable.rb @@ -21,7 +21,7 @@ module Ci }, _suffix: true scope :expired_before, -> (timestamp) { where(arel_table[:expire_at].lt(timestamp)) } - scope :expired, -> (limit) { expired_before(Time.current).limit(limit) } + scope :expired, -> { expired_before(Time.current) } scope :project_id_in, ->(ids) { where(project_id: ids) } end diff --git a/app/models/user.rb b/app/models/user.rb index 40735f85ded..ba187ddfc23 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -2302,12 +2302,7 @@ class User < ApplicationRecord .merge(search_members) .shortest_traversal_ids_prefixes - # Use efficient btree index to perform search - if Feature.enabled?(:ci_owned_runners_unnest_index, self) - Ci::NamespaceMirror.contains_traversal_ids(traversal_ids) - else - Ci::NamespaceMirror.contains_any_of_namespaces(traversal_ids.map(&:last)) - end + Ci::NamespaceMirror.contains_traversal_ids(traversal_ids) end end diff --git a/app/services/ci/pipeline_artifacts/destroy_all_expired_service.rb b/app/services/ci/pipeline_artifacts/destroy_all_expired_service.rb index 7b6590a117c..17c039885e5 100644 --- a/app/services/ci/pipeline_artifacts/destroy_all_expired_service.rb +++ b/app/services/ci/pipeline_artifacts/destroy_all_expired_service.rb @@ -25,7 +25,7 @@ module Ci private def destroy_artifacts_batch - artifacts = ::Ci::PipelineArtifact.unlocked.expired(BATCH_SIZE).to_a + artifacts = ::Ci::PipelineArtifact.unlocked.expired.limit(BATCH_SIZE).to_a return false if artifacts.empty? artifacts.each(&:destroy!) diff --git a/config/feature_flags/development/ci_owned_runners_unnest_index.yml b/config/feature_flags/development/ci_owned_runners_unnest_index.yml deleted file mode 100644 index 3ac4fdf8175..00000000000 --- a/config/feature_flags/development/ci_owned_runners_unnest_index.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: ci_owned_runners_unnest_index -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/83843 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/357869 -milestone: '14.10' -type: development -group: group::sharding -default_enabled: true diff --git a/lib/backup/repositories.rb b/lib/backup/repositories.rb index 29b20bd2e73..82a711add7f 100644 --- a/lib/backup/repositories.rb +++ b/lib/backup/repositories.rb @@ -87,7 +87,6 @@ module Backup PoolRepository.includes(:source_project).find_each do |pool| progress.puts " - Object pool #{pool.disk_path}..." - pool.source_project ||= pool.member_projects.first&.root_of_fork_network unless pool.source_project progress.puts " - Object pool #{pool.disk_path}... " + "[SKIPPED]".color(:cyan) next diff --git a/spec/lib/backup/repositories_spec.rb b/spec/lib/backup/repositories_spec.rb index 211b0d91f9f..f05585b5b89 100644 --- a/spec/lib/backup/repositories_spec.rb +++ b/spec/lib/backup/repositories_spec.rb @@ -163,7 +163,7 @@ RSpec.describe Backup::Repositories do expect(pool_repository.object_pool.exists?).to be(true) end - it 'skips pools with no source project, :sidekiq_might_not_need_inline' do + it 'skips pools when no source project is found', :sidekiq_might_not_need_inline do pool_repository = create(:pool_repository, state: :obsolete) pool_repository.update_column(:source_project_id, nil) diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 8dc041814fa..9ab17ea6af7 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -4839,9 +4839,9 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe '#has_expired_test_reports?' do - subject { pipeline_with_test_report.has_expired_test_reports? } + subject { pipeline.has_expired_test_reports? } - let(:pipeline_with_test_report) { create(:ci_pipeline, :with_test_reports) } + let(:pipeline) { create(:ci_pipeline, :success, :with_test_reports) } context 'when artifacts are not expired' do it { is_expected.to be_falsey } @@ -4849,11 +4849,23 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do context 'when artifacts are expired' do before do - pipeline_with_test_report.job_artifacts.first.update!(expire_at: Date.yesterday) + pipeline.job_artifacts.first.update!(expire_at: Date.yesterday) end it { is_expected.to be_truthy } end + + context 'when the pipeline is still running' do + let(:pipeline) { create(:ci_pipeline, :running) } + + it { is_expected.to be_falsey } + end + + context 'when the pipeline is completed without test reports' do + let(:pipeline) { create(:ci_pipeline, :success) } + + it { is_expected.to be_falsey } + end end it_behaves_like 'it has loose foreign keys' do diff --git a/spec/models/concerns/ci/artifactable_spec.rb b/spec/models/concerns/ci/artifactable_spec.rb index 62fc689a9ca..b27a4d0dcc1 100644 --- a/spec/models/concerns/ci/artifactable_spec.rb +++ b/spec/models/concerns/ci/artifactable_spec.rb @@ -68,8 +68,8 @@ RSpec.describe Ci::Artifactable do end describe '.expired' do - it 'returns a limited number of expired artifacts' do - expect(Ci::JobArtifact.expired(1).order_id_asc).to eq([recently_expired_artifact]) + it 'returns all expired artifacts' do + expect(Ci::JobArtifact.expired).to contain_exactly(recently_expired_artifact, later_expired_artifact) end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index ac085d71761..d46bdfa65a8 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -4374,14 +4374,6 @@ RSpec.describe User do it_behaves_like '#ci_owned_runners' end - - context 'when FF ci_owned_runners_unnest_index is disabled uses GIN index' do - before do - stub_feature_flags(ci_owned_runners_unnest_index: false) - end - - it_behaves_like '#ci_owned_runners' - end end describe '#projects_with_reporter_access_limited_to' do diff --git a/workhorse/internal/artifacts/entry_test.go b/workhorse/internal/artifacts/entry_test.go index 800125eec91..36abb1d322f 100644 --- a/workhorse/internal/artifacts/entry_test.go +++ b/workhorse/internal/artifacts/entry_test.go @@ -63,9 +63,7 @@ func TestDownloadingFromValidArchive(t *testing.T) { } func TestDownloadingFromValidHTTPArchive(t *testing.T) { - tempDir, err := ioutil.TempDir("", "uploads") - require.NoError(t, err) - defer os.RemoveAll(tempDir) + tempDir := t.TempDir() f, err := os.Create(filepath.Join(tempDir, "archive.zip")) require.NoError(t, err) @@ -121,9 +119,7 @@ func TestIncompleteApiResponse(t *testing.T) { } func TestDownloadingFromNonExistingHTTPArchive(t *testing.T) { - tempDir, err := ioutil.TempDir("", "uploads") - require.NoError(t, err) - defer os.RemoveAll(tempDir) + tempDir := t.TempDir() fileServer := httptest.NewServer(http.FileServer(http.Dir(tempDir))) defer fileServer.Close() diff --git a/workhorse/internal/git/upload-pack_test.go b/workhorse/internal/git/upload-pack_test.go index 9ffc7117790..9dfeb23e9e2 100644 --- a/workhorse/internal/git/upload-pack_test.go +++ b/workhorse/internal/git/upload-pack_test.go @@ -8,7 +8,6 @@ import ( "io/ioutil" "net" "net/http/httptest" - "os" "path/filepath" "testing" "time" @@ -75,8 +74,7 @@ func TestUploadPackTimesOut(t *testing.T) { func startSmartHTTPServer(t testing.TB, s gitalypb.SmartHTTPServiceServer) string { t.Helper() - tmp, err := ioutil.TempDir("", "") - require.NoError(t, err) + tmp := t.TempDir() socket := filepath.Join(tmp, "gitaly.sock") ln, err := net.Listen("unix", socket) @@ -90,7 +88,6 @@ func startSmartHTTPServer(t testing.TB, s gitalypb.SmartHTTPServiceServer) strin t.Cleanup(func() { srv.GracefulStop() - require.NoError(t, os.RemoveAll(tmp), "error removing temp dir %q", tmp) }) return fmt.Sprintf("%s://%s", ln.Addr().Network(), ln.Addr().String()) diff --git a/workhorse/internal/httprs/httprs_test.go b/workhorse/internal/httprs/httprs_test.go index e26d2d21215..cc3af504cde 100644 --- a/workhorse/internal/httprs/httprs_test.go +++ b/workhorse/internal/httprs/httprs_test.go @@ -111,11 +111,9 @@ func newRSFactory(flags int) RSFactory { func TestHttpWebServer(t *testing.T) { Convey("Scenario: testing WebServer", t, func() { - dir, err := ioutil.TempDir("", "webserver") - So(err, ShouldBeNil) - defer os.RemoveAll(dir) + dir := t.TempDir() - err = ioutil.WriteFile(filepath.Join(dir, "file"), make([]byte, 10000), 0755) + err := ioutil.WriteFile(filepath.Join(dir, "file"), make([]byte, 10000), 0755) So(err, ShouldBeNil) server := httptest.NewServer(http.FileServer(http.Dir(dir))) diff --git a/workhorse/internal/staticpages/deploy_page_test.go b/workhorse/internal/staticpages/deploy_page_test.go index bc413880184..56667a45807 100644 --- a/workhorse/internal/staticpages/deploy_page_test.go +++ b/workhorse/internal/staticpages/deploy_page_test.go @@ -4,7 +4,6 @@ import ( "io/ioutil" "net/http" "net/http/httptest" - "os" "path/filepath" "testing" @@ -14,11 +13,7 @@ import ( ) func TestIfNoDeployPageExist(t *testing.T) { - dir, err := ioutil.TempDir("", "deploy") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(dir) + dir := t.TempDir() w := httptest.NewRecorder() @@ -33,11 +28,7 @@ func TestIfNoDeployPageExist(t *testing.T) { } func TestIfDeployPageExist(t *testing.T) { - dir, err := ioutil.TempDir("", "deploy") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(dir) + dir := t.TempDir() deployPage := "DEPLOY" ioutil.WriteFile(filepath.Join(dir, "index.html"), []byte(deployPage), 0600) diff --git a/workhorse/internal/staticpages/error_pages_test.go b/workhorse/internal/staticpages/error_pages_test.go index c9927668fcc..80a6e7920df 100644 --- a/workhorse/internal/staticpages/error_pages_test.go +++ b/workhorse/internal/staticpages/error_pages_test.go @@ -5,7 +5,6 @@ import ( "io/ioutil" "net/http" "net/http/httptest" - "os" "path/filepath" "testing" @@ -15,11 +14,7 @@ import ( ) func TestIfErrorPageIsPresented(t *testing.T) { - dir, err := ioutil.TempDir("", "error_page") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(dir) + dir := t.TempDir() errorPage := "ERROR" ioutil.WriteFile(filepath.Join(dir, "404.html"), []byte(errorPage), 0600) @@ -42,11 +37,7 @@ func TestIfErrorPageIsPresented(t *testing.T) { } func TestIfErrorPassedIfNoErrorPageIsFound(t *testing.T) { - dir, err := ioutil.TempDir("", "error_page") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(dir) + dir := t.TempDir() w := httptest.NewRecorder() errorResponse := "ERROR" @@ -63,11 +54,7 @@ func TestIfErrorPassedIfNoErrorPageIsFound(t *testing.T) { } func TestIfErrorPageIsIgnoredInDevelopment(t *testing.T) { - dir, err := ioutil.TempDir("", "error_page") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(dir) + dir := t.TempDir() errorPage := "ERROR" ioutil.WriteFile(filepath.Join(dir, "500.html"), []byte(errorPage), 0600) @@ -86,11 +73,7 @@ func TestIfErrorPageIsIgnoredInDevelopment(t *testing.T) { } func TestIfErrorPageIsIgnoredIfCustomError(t *testing.T) { - dir, err := ioutil.TempDir("", "error_page") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(dir) + dir := t.TempDir() errorPage := "ERROR" ioutil.WriteFile(filepath.Join(dir, "500.html"), []byte(errorPage), 0600) @@ -121,11 +104,7 @@ func TestErrorPageInterceptedByContentType(t *testing.T) { } for _, tc := range testCases { - dir, err := ioutil.TempDir("", "error_page") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(dir) + dir := t.TempDir() errorPage := "ERROR" ioutil.WriteFile(filepath.Join(dir, "500.html"), []byte(errorPage), 0600) diff --git a/workhorse/internal/staticpages/servefile_test.go b/workhorse/internal/staticpages/servefile_test.go index 67675beccf8..de38474a683 100644 --- a/workhorse/internal/staticpages/servefile_test.go +++ b/workhorse/internal/staticpages/servefile_test.go @@ -6,7 +6,6 @@ import ( "io/ioutil" "net/http" "net/http/httptest" - "os" "path/filepath" "testing" @@ -26,11 +25,7 @@ func TestServingNonExistingFile(t *testing.T) { } func TestServingDirectory(t *testing.T) { - dir, err := ioutil.TempDir("", "deploy") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(dir) + dir := t.TempDir() httpRequest, _ := http.NewRequest("GET", "/file", nil) w := httptest.NewRecorder() @@ -64,11 +59,7 @@ func TestExecutingHandlerWhenNoFileFound(t *testing.T) { } func TestServingTheActualFile(t *testing.T) { - dir, err := ioutil.TempDir("", "deploy") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(dir) + dir := t.TempDir() httpRequest, _ := http.NewRequest("GET", "/file", nil) @@ -121,11 +112,7 @@ func TestExcludedPaths(t *testing.T) { } func testServingThePregzippedFile(t *testing.T, enableGzip bool) { - dir, err := ioutil.TempDir("", "deploy") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(dir) + dir := t.TempDir() httpRequest, _ := http.NewRequest("GET", "/file", nil) diff --git a/workhorse/internal/upload/artifacts_store_test.go b/workhorse/internal/upload/artifacts_store_test.go index 97e66fc37a4..0a4f5da0197 100644 --- a/workhorse/internal/upload/artifacts_store_test.go +++ b/workhorse/internal/upload/artifacts_store_test.go @@ -10,7 +10,6 @@ import ( "mime/multipart" "net/http" "net/http/httptest" - "os" "testing" "time" @@ -56,16 +55,11 @@ func testUploadArtifactsFromTestZip(t *testing.T, ts *httptest.Server) *httptest } func TestUploadHandlerSendingToExternalStorage(t *testing.T) { - tempPath, err := ioutil.TempDir("", "uploads") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(tempPath) + tempPath := t.TempDir() archiveData, md5 := createTestZipArchive(t) - archiveFile, err := ioutil.TempFile("", "artifact.zip") + archiveFile, err := ioutil.TempFile(tempPath, "artifact.zip") require.NoError(t, err) - defer os.Remove(archiveFile.Name()) _, err = archiveFile.Write(archiveData) require.NoError(t, err) archiveFile.Close() @@ -135,11 +129,7 @@ func TestUploadHandlerSendingToExternalStorage(t *testing.T) { } func TestUploadHandlerSendingToExternalStorageAndStorageServerUnreachable(t *testing.T) { - tempPath, err := ioutil.TempDir("", "uploads") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(tempPath) + tempPath := t.TempDir() responseProcessor := func(w http.ResponseWriter, r *http.Request) { t.Fatal("it should not be called") @@ -161,11 +151,7 @@ func TestUploadHandlerSendingToExternalStorageAndStorageServerUnreachable(t *tes } func TestUploadHandlerSendingToExternalStorageAndInvalidURLIsUsed(t *testing.T) { - tempPath, err := ioutil.TempDir("", "uploads") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(tempPath) + tempPath := t.TempDir() responseProcessor := func(w http.ResponseWriter, r *http.Request) { t.Fatal("it should not be called") diff --git a/workhorse/internal/upload/artifacts_upload_test.go b/workhorse/internal/upload/artifacts_upload_test.go index 96eb3810673..0837509fadb 100644 --- a/workhorse/internal/upload/artifacts_upload_test.go +++ b/workhorse/internal/upload/artifacts_upload_test.go @@ -130,8 +130,7 @@ type testServer struct { } func setupWithTmpPath(t *testing.T, filename string, includeFormat bool, format string, authResponse *api.Response, bodyProcessor func(w http.ResponseWriter, r *http.Request)) *testServer { - tempPath, err := ioutil.TempDir("", "uploads") - require.NoError(t, err) + tempPath := t.TempDir() if authResponse == nil { authResponse = &api.Response{TempPath: tempPath} @@ -147,7 +146,6 @@ func setupWithTmpPath(t *testing.T, filename string, includeFormat bool, format cleanup := func() { ts.Close() - require.NoError(t, os.RemoveAll(tempPath)) require.NoError(t, writer.Close()) } @@ -292,8 +290,7 @@ func TestUploadFormProcessing(t *testing.T) { } func TestLsifFileProcessing(t *testing.T) { - tempPath, err := ioutil.TempDir("", "uploads") - require.NoError(t, err) + tempPath := t.TempDir() s := setupWithTmpPath(t, "file", true, "zip", &api.Response{TempPath: tempPath, ProcessLsif: true}, nil) defer s.cleanup() @@ -312,8 +309,7 @@ func TestLsifFileProcessing(t *testing.T) { } func TestInvalidLsifFileProcessing(t *testing.T) { - tempPath, err := ioutil.TempDir("", "uploads") - require.NoError(t, err) + tempPath := t.TempDir() s := setupWithTmpPath(t, "file", true, "zip", &api.Response{TempPath: tempPath, ProcessLsif: true}, nil) defer s.cleanup() diff --git a/workhorse/internal/upload/destination/destination_test.go b/workhorse/internal/upload/destination/destination_test.go index 66df313cf9b..6ebe163468b 100644 --- a/workhorse/internal/upload/destination/destination_test.go +++ b/workhorse/internal/upload/destination/destination_test.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "io/ioutil" "os" "path" "strconv" @@ -43,9 +42,7 @@ func TestUploadWrongSize(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - tmpFolder, err := ioutil.TempDir("", "workhorse-test-tmp") - require.NoError(t, err) - defer os.RemoveAll(tmpFolder) + tmpFolder := t.TempDir() opts := &destination.UploadOpts{LocalTempPath: tmpFolder} fh, err := destination.Upload(ctx, strings.NewReader(test.ObjectContent), test.ObjectSize+1, "upload", opts) @@ -59,9 +56,7 @@ func TestUploadWithKnownSizeExceedLimit(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - tmpFolder, err := ioutil.TempDir("", "workhorse-test-tmp") - require.NoError(t, err) - defer os.RemoveAll(tmpFolder) + tmpFolder := t.TempDir() opts := &destination.UploadOpts{LocalTempPath: tmpFolder, MaximumSize: test.ObjectSize - 1} fh, err := destination.Upload(ctx, strings.NewReader(test.ObjectContent), test.ObjectSize, "upload", opts) @@ -75,9 +70,7 @@ func TestUploadWithUnknownSizeExceedLimit(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - tmpFolder, err := ioutil.TempDir("", "workhorse-test-tmp") - require.NoError(t, err) - defer os.RemoveAll(tmpFolder) + tmpFolder := t.TempDir() opts := &destination.UploadOpts{LocalTempPath: tmpFolder, MaximumSize: test.ObjectSize - 1} fh, err := destination.Upload(ctx, strings.NewReader(test.ObjectContent), -1, "upload", opts) @@ -139,9 +132,7 @@ func TestUpload(t *testing.T) { remoteMultipart ) - tmpFolder, err := ioutil.TempDir("", "workhorse-test-tmp") - require.NoError(t, err) - defer os.RemoveAll(tmpFolder) + tmpFolder := t.TempDir() tests := []struct { name string @@ -301,8 +292,7 @@ func TestUploadWithS3WorkhorseClient(t *testing.T) { } func TestUploadWithAzureWorkhorseClient(t *testing.T) { - mux, bucketDir, cleanup := test.SetupGoCloudFileBucket(t, "azblob") - defer cleanup() + mux, bucketDir := test.SetupGoCloudFileBucket(t, "azblob") ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -427,10 +417,6 @@ func TestUploadRemoteFileWithLimit(t *testing.T) { var opts destination.UploadOpts for _, remoteType := range remoteTypes { - tmpFolder, err := ioutil.TempDir("", "workhorse-test-tmp") - require.NoError(t, err) - defer os.RemoveAll(tmpFolder) - osStub, ts := test.StartObjectStore() defer ts.Close() diff --git a/workhorse/internal/upload/destination/objectstore/gocloud_object_test.go b/workhorse/internal/upload/destination/objectstore/gocloud_object_test.go index 57b3a35b41e..55d886087be 100644 --- a/workhorse/internal/upload/destination/objectstore/gocloud_object_test.go +++ b/workhorse/internal/upload/destination/objectstore/gocloud_object_test.go @@ -15,8 +15,7 @@ import ( ) func TestGoCloudObjectUpload(t *testing.T) { - mux, _, cleanup := test.SetupGoCloudFileBucket(t, "azuretest") - defer cleanup() + mux, _ := test.SetupGoCloudFileBucket(t, "azuretest") ctx, cancel := context.WithCancel(context.Background()) deadline := time.Now().Add(testTimeout) diff --git a/workhorse/internal/upload/destination/objectstore/s3_object_test.go b/workhorse/internal/upload/destination/objectstore/s3_object_test.go index b81b0ae2024..0ed14a2e844 100644 --- a/workhorse/internal/upload/destination/objectstore/s3_object_test.go +++ b/workhorse/internal/upload/destination/objectstore/s3_object_test.go @@ -4,8 +4,6 @@ import ( "context" "fmt" "io" - "io/ioutil" - "os" "path/filepath" "strings" "sync" @@ -47,9 +45,7 @@ func TestS3ObjectUpload(t *testing.T) { defer ts.Close() deadline := time.Now().Add(testTimeout) - tmpDir, err := ioutil.TempDir("", "workhorse-test-") - require.NoError(t, err) - defer os.Remove(tmpDir) + tmpDir := t.TempDir() objectName := filepath.Join(tmpDir, "s3-test-data") ctx, cancel := context.WithCancel(context.Background()) @@ -87,9 +83,7 @@ func TestConcurrentS3ObjectUpload(t *testing.T) { defer artifactsServer.Close() deadline := time.Now().Add(testTimeout) - tmpDir, err := ioutil.TempDir("", "workhorse-test-") - require.NoError(t, err) - defer os.Remove(tmpDir) + tmpDir := t.TempDir() var wg sync.WaitGroup @@ -136,9 +130,7 @@ func TestS3ObjectUploadCancel(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) deadline := time.Now().Add(testTimeout) - tmpDir, err := ioutil.TempDir("", "workhorse-test-") - require.NoError(t, err) - defer os.Remove(tmpDir) + tmpDir := t.TempDir() objectName := filepath.Join(tmpDir, "s3-test-data") @@ -160,9 +152,7 @@ func TestS3ObjectUploadLimitReached(t *testing.T) { defer ts.Close() deadline := time.Now().Add(testTimeout) - tmpDir, err := ioutil.TempDir("", "workhorse-test-") - require.NoError(t, err) - defer os.Remove(tmpDir) + tmpDir := t.TempDir() objectName := filepath.Join(tmpDir, "s3-test-data") object, err := objectstore.NewS3Object(objectName, creds, config) diff --git a/workhorse/internal/upload/destination/objectstore/test/gocloud_stub.go b/workhorse/internal/upload/destination/objectstore/test/gocloud_stub.go index cf22075e407..bff0eabaee5 100644 --- a/workhorse/internal/upload/destination/objectstore/test/gocloud_stub.go +++ b/workhorse/internal/upload/destination/objectstore/test/gocloud_stub.go @@ -2,9 +2,7 @@ package test import ( "context" - "io/ioutil" "net/url" - "os" "testing" "github.com/stretchr/testify/require" @@ -20,18 +18,14 @@ func (o *dirOpener) OpenBucketURL(ctx context.Context, u *url.URL) (*blob.Bucket return fileblob.OpenBucket(o.tmpDir, nil) } -func SetupGoCloudFileBucket(t *testing.T, scheme string) (m *blob.URLMux, bucketDir string, cleanup func()) { - tmpDir, err := ioutil.TempDir("", "") - require.NoError(t, err) +func SetupGoCloudFileBucket(t *testing.T, scheme string) (m *blob.URLMux, bucketDir string) { + tmpDir := t.TempDir() mux := new(blob.URLMux) fake := &dirOpener{tmpDir: tmpDir} mux.RegisterBucket(scheme, fake) - cleanup = func() { - os.RemoveAll(tmpDir) - } - return mux, tmpDir, cleanup + return mux, tmpDir } func GoCloudObjectExists(t *testing.T, bucketDir string, objectName string) { diff --git a/workhorse/internal/upload/destination/objectstore/test/s3_stub.go b/workhorse/internal/upload/destination/objectstore/test/s3_stub.go index 6b83426b852..9a8be7d6089 100644 --- a/workhorse/internal/upload/destination/objectstore/test/s3_stub.go +++ b/workhorse/internal/upload/destination/objectstore/test/s3_stub.go @@ -124,13 +124,10 @@ func S3ObjectDoesNotExist(t *testing.T, sess *session.Session, config config.S3C } func downloadObject(t *testing.T, sess *session.Session, config config.S3Config, objectName string, handler func(tmpfile *os.File, numBytes int64, err error)) { - tmpDir, err := ioutil.TempDir("", "workhorse-test-") - require.NoError(t, err) - defer os.Remove(tmpDir) + tmpDir := t.TempDir() tmpfile, err := ioutil.TempFile(tmpDir, "s3-output") require.NoError(t, err) - defer os.Remove(tmpfile.Name()) downloadSvc := s3manager.NewDownloader(sess) numBytes, err := downloadSvc.Download(tmpfile, &s3.GetObjectInput{ diff --git a/workhorse/internal/upload/destination/upload_opts_test.go b/workhorse/internal/upload/destination/upload_opts_test.go index 24a372495c6..fd9e56db194 100644 --- a/workhorse/internal/upload/destination/upload_opts_test.go +++ b/workhorse/internal/upload/destination/upload_opts_test.go @@ -283,8 +283,7 @@ func TestUseWorkhorseClientEnabled(t *testing.T) { } func TestGoCloudConfig(t *testing.T) { - mux, _, cleanup := test.SetupGoCloudFileBucket(t, "azblob") - defer cleanup() + mux, _ := test.SetupGoCloudFileBucket(t, "azblob") tests := []struct { name string