From 1f3b8d4da4bdf76cd6060ed652bafeb372af5bfe Mon Sep 17 00:00:00 2001 From: Jarek Kowalski Date: Sat, 16 Jan 2021 18:21:16 -0800 Subject: [PATCH] upgrade linter to 1.35 (#786) * lint: added test that enforces Makefile and GH action linter versions are in sync * workaround for linter gomnd problem - https://github.com/golangci/golangci-lint/issues/1653 --- .github/workflows/lint.yml | 23 +++------- .golangci.yml | 5 +++ fs/cachefs/cache_test.go | 11 ++++- fs/localfs/local_fs_test.go | 2 + internal/apiclient/apiclient.go | 1 - internal/blobtesting/faulty.go | 2 + internal/editor/editor.go | 2 +- internal/repotesting/repotesting.go | 12 +++++ internal/serverapi/serverapi.go | 3 +- internal/testutil/retriable_t.go | 2 + repo/blob/azure/azure_storage.go | 7 ++- repo/blob/azure/azure_storage_test.go | 4 ++ repo/blob/b2/b2_storage_test.go | 2 + .../filesystem/filesystem_storage_test.go | 2 + repo/blob/gcs/gcs_storage.go | 3 +- repo/blob/s3/s3_storage.go | 2 +- repo/blob/s3/s3_storage_test.go | 12 ++--- repo/blob/sftp/sftp_storage.go | 2 +- repo/blob/sftp/sftp_storage_test.go | 6 +++ repo/blob/webdav/webdav_storage_test.go | 2 + repo/compression/compressor_s2.go | 9 +++- repo/content/blob_crypto.go | 2 +- repo/content/builder.go | 1 - repo/content/content_cache_test.go | 4 ++ repo/content/content_formatter_test.go | 2 + repo/content/content_manager.go | 1 - repo/content/content_manager_lock_free.go | 2 +- repo/content/content_manager_own_writes.go | 3 +- repo/content/content_manager_test.go | 17 +++++++ repo/content/format.go | 12 +++-- repo/content/index_blob_manager_test.go | 8 ++++ repo/content/merged_test.go | 2 + repo/encryption/deprecated_ctr_encryptor.go | 12 +++-- repo/encryption/encryption_test.go | 2 + repo/encryption/null_encryptor.go | 3 +- repo/manifest/manifest_manager_test.go | 2 + repo/object/object_manager_test.go | 6 +++ repo/repository_test.go | 2 + repo/splitter/splitter.go | 2 +- snapshot/policy/actions_policy.go | 1 - snapshot/policy/policy_manager_test.go | 4 ++ snapshot/snapshot_test.go | 6 +++ snapshot/snapshotfs/all_sources.go | 2 +- snapshot/snapshotfs/source_directories.go | 2 +- snapshot/snapshotfs/source_snapshots.go | 2 +- snapshot/snapshotfs/upload_progress.go | 3 +- snapshot/snapshotgc/gc.go | 1 - .../snapshotmaintenance_test.go | 2 + tests/end_to_end_test/server_start_test.go | 2 + .../end_to_end_test/snapshot_actions_test.go | 4 ++ tests/end_to_end_test/snapshot_delete_test.go | 4 ++ tests/end_to_end_test/snapshot_fail_test.go | 8 +++- tests/endurance_test/endurance_test.go | 14 ++++++ .../repository_stress_test.go | 22 +++++++++ tests/robustness/engine/action.go | 2 +- tests/robustness/engine/engine_test.go | 2 + tests/stress_test/stress_test.go | 2 + tests/testenv/cli_test_env.go | 24 +++++++++- tests/testenv/faketimeserver.go | 2 +- tests/tools/fio/workload_test.go | 11 +++++ tools/linter_version_test.go | 45 +++++++++++++++++++ tools/tools.mk | 2 +- 62 files changed, 301 insertions(+), 63 deletions(-) create mode 100644 tools/linter_version_test.go diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 9b99fa435..8d6163dff 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -5,21 +5,12 @@ on: branches: [ master ] jobs: - - build: - name: Build + golangci: + name: Lint runs-on: ubuntu-latest steps: - - - name: Set up Go - uses: actions/setup-go@v2 - with: - go-version: ^1.14 - id: go - - - name: Check out code into the Go module directory - uses: actions/checkout@v2 - - - name: Lint - run: | - make lint + - uses: actions/checkout@v2 + - name: golangci-lint + uses: golangci/golangci-lint-action@v2 + with: + version: v1.35 diff --git a/.golangci.yml b/.golangci.yml index 2609645ad..8bf0e47b4 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -55,6 +55,9 @@ run: issues: exclude-use-default: false exclude-rules: + - path: cli + linters: + - forbidigo # allow fmt.Printf in the CLI - path: _test\.go|testing|tests|test_env|fshasher linters: - gomnd @@ -64,6 +67,8 @@ issues: - gosec - nestif - wrapcheck + - forbidigo + - nolintlint - text: "Magic number: 1e" linters: - gomnd diff --git a/fs/cachefs/cache_test.go b/fs/cachefs/cache_test.go index 250130cf5..c3a832072 100644 --- a/fs/cachefs/cache_test.go +++ b/fs/cachefs/cache_test.go @@ -64,6 +64,8 @@ type cacheVerifier struct { } func (cv *cacheVerifier) verifyCacheMiss(t *testing.T, id string) { + t.Helper() + actual := cv.cacheSource.callCounter[id] expected := cv.lastCallCounter[id] + 1 @@ -75,6 +77,8 @@ func (cv *cacheVerifier) verifyCacheMiss(t *testing.T, id string) { } func (cv *cacheVerifier) verifyCacheHit(t *testing.T, id string) { + t.Helper() + if !reflect.DeepEqual(cv.lastCallCounter, cv.cacheSource.callCounter) { t.Errorf(errorPrefix()+" unexpected call counters for %v, got %v, expected %v", id, cv.cacheSource.callCounter, cv.lastCallCounter) } @@ -83,9 +87,12 @@ func (cv *cacheVerifier) verifyCacheHit(t *testing.T, id string) { } func (cv *cacheVerifier) verifyCacheOrdering(t *testing.T, expectedOrdering ...string) { - var actualOrdering []string + t.Helper() - var totalDirectoryEntries, totalDirectories int + var ( + actualOrdering []string + totalDirectoryEntries, totalDirectories int + ) for e := cv.cache.head; e != nil; e = e.next { actualOrdering = append(actualOrdering, e.id) diff --git a/fs/localfs/local_fs_test.go b/fs/localfs/local_fs_test.go index aa7ad15a3..4057754ae 100644 --- a/fs/localfs/local_fs_test.go +++ b/fs/localfs/local_fs_test.go @@ -103,6 +103,8 @@ func TestFiles(t *testing.T) { } func verifyChild(t *testing.T, dir fs.Directory) { + t.Helper() + ctx := testlogging.Context(t) child, err := dir.Child(ctx, "f3") diff --git a/internal/apiclient/apiclient.go b/internal/apiclient/apiclient.go index 4628a459e..99e098975 100644 --- a/internal/apiclient/apiclient.go +++ b/internal/apiclient/apiclient.go @@ -130,7 +130,6 @@ type Options struct { } // NewKopiaAPIClient creates a client for connecting to Kopia HTTP API. -// nolint:gocritic func NewKopiaAPIClient(options Options) (*KopiaAPIClient, error) { var transport http.RoundTripper diff --git a/internal/blobtesting/faulty.go b/internal/blobtesting/faulty.go index 183b0db3a..693420ef9 100644 --- a/internal/blobtesting/faulty.go +++ b/internal/blobtesting/faulty.go @@ -158,6 +158,8 @@ func (s *FaultyStorage) getNextFault(ctx context.Context, method string, args .. // VerifyAllFaultsExercised fails the test if some faults have not been exercised. func (s *FaultyStorage) VerifyAllFaultsExercised(t *testing.T) { + t.Helper() + if len(s.Faults) != 0 { t.Fatalf("not all defined faults have been hit: %#v", s.Faults) } diff --git a/internal/editor/editor.go b/internal/editor/editor.go index 1368fb645..793dbf25b 100644 --- a/internal/editor/editor.go +++ b/internal/editor/editor.go @@ -51,7 +51,7 @@ func EditLoop(ctx context.Context, fname, initial string, parse func(updated str } log(ctx).Errorf("%v", err) - fmt.Print("Reopen editor to fix? (Y/n) ") + fmt.Print("Reopen editor to fix? (Y/n) ") //nolint:forbidigo var shouldReopen string diff --git a/internal/repotesting/repotesting.go b/internal/repotesting/repotesting.go index 777b245d4..d6efc6fb7 100644 --- a/internal/repotesting/repotesting.go +++ b/internal/repotesting/repotesting.go @@ -35,6 +35,8 @@ type Options struct { // Setup sets up a test environment. func (e *Environment) Setup(t *testing.T, opts ...Options) *Environment { + t.Helper() + ctx := testlogging.Context(t) e.configDir = t.TempDir() e.storageDir = t.TempDir() @@ -90,6 +92,8 @@ func (e *Environment) Setup(t *testing.T, opts ...Options) *Environment { // Close closes testing environment. func (e *Environment) Close(ctx context.Context, t *testing.T) { + t.Helper() + if err := e.Repository.Close(ctx); err != nil { t.Fatalf("unable to close: %v", err) } @@ -112,6 +116,8 @@ func (e *Environment) configFile() string { // MustReopen closes and reopens the repository. func (e *Environment) MustReopen(t *testing.T, openOpts ...func(*repo.Options)) { + t.Helper() + err := e.Repository.Close(testlogging.Context(t)) if err != nil { t.Fatalf("close error: %v", err) @@ -127,6 +133,8 @@ func (e *Environment) MustReopen(t *testing.T, openOpts ...func(*repo.Options)) // MustOpenAnother opens another repository backend by the same storage. func (e *Environment) MustOpenAnother(t *testing.T) repo.Repository { + t.Helper() + rep2, err := repo.Open(testlogging.Context(t), e.configFile(), masterPassword, &repo.Options{}) if err != nil { t.Fatalf("err: %v", err) @@ -142,6 +150,8 @@ func (e *Environment) MustOpenAnother(t *testing.T) repo.Repository { // MustConnectOpenAnother opens another repository backend by the same storage, // with independent config and cache options. func (e *Environment) MustConnectOpenAnother(t *testing.T, openOpts ...func(*repo.Options)) repo.Repository { + t.Helper() + ctx := testlogging.Context(t) st, err := filesystem.New(ctx, &filesystem.Options{ @@ -172,6 +182,8 @@ func (e *Environment) MustConnectOpenAnother(t *testing.T, openOpts ...func(*rep // VerifyBlobCount verifies that the underlying storage contains the specified number of blobs. func (e *Environment) VerifyBlobCount(t *testing.T, want int) { + t.Helper() + var got int _ = e.Repository.Blobs.ListBlobs(testlogging.Context(t), "", func(_ blob.Metadata) error { diff --git a/internal/serverapi/serverapi.go b/internal/serverapi/serverapi.go index 116ec9938..9712df76d 100644 --- a/internal/serverapi/serverapi.go +++ b/internal/serverapi/serverapi.go @@ -63,8 +63,7 @@ type PoliciesResponse struct { } // Empty represents empty request/response. -type Empty struct { -} +type Empty struct{} // APIErrorCode indicates machine-readable error code returned in API responses. type APIErrorCode string diff --git a/internal/testutil/retriable_t.go b/internal/testutil/retriable_t.go index 0b067a91e..7cf83e9af 100644 --- a/internal/testutil/retriable_t.go +++ b/internal/testutil/retriable_t.go @@ -89,6 +89,8 @@ func (t *RetriableT) Skipf(msg string, args ...interface{}) { // Retry invokes the provided test multiple tests until it succeeds. func Retry(t *testing.T, testFun func(t *RetriableT)) { + t.Helper() + nextSleepTime := initialSleep for att := 1; att <= maxRetries; att++ { diff --git a/repo/blob/azure/azure_storage.go b/repo/blob/azure/azure_storage.go index 11d3f1095..6aabb3a33 100644 --- a/repo/blob/azure/azure_storage.go +++ b/repo/blob/azure/azure_storage.go @@ -6,6 +6,7 @@ "fmt" "io" "io/ioutil" + "net/http" "time" "github.com/Azure/azure-storage-blob-go/azblob" @@ -100,11 +101,13 @@ func isRetriableError(err error) bool { var me azblob.ResponseError if errors.As(err, &me) { - if me.Response() == nil { + r := me.Response() //nolint:bodyclose + if r == nil { return true } + // retry on server errors, not on client errors - return me.Response().StatusCode >= 500 + return r.StatusCode >= http.StatusInternalServerError } // https://pkg.go.dev/gocloud.dev/gcerrors?tab=doc#ErrorCode diff --git a/repo/blob/azure/azure_storage_test.go b/repo/blob/azure/azure_storage_test.go index d5c4983b8..4153869f9 100644 --- a/repo/blob/azure/azure_storage_test.go +++ b/repo/blob/azure/azure_storage_test.go @@ -24,6 +24,8 @@ ) func getEnvOrSkip(t *testing.T, name string) string { + t.Helper() + value := os.Getenv(name) if value == "" { t.Skip(fmt.Sprintf("%s not provided", name)) @@ -33,6 +35,8 @@ func getEnvOrSkip(t *testing.T, name string) string { } func createContainer(t *testing.T, container, storageAccount, storageKey string) { + t.Helper() + credential, err := azblob.NewSharedKeyCredential(storageAccount, storageKey) if err != nil { t.Fatalf("failed to create Azure credentials: %v", err) diff --git a/repo/blob/b2/b2_storage_test.go b/repo/blob/b2/b2_storage_test.go index b453ff9c7..89d6188de 100644 --- a/repo/blob/b2/b2_storage_test.go +++ b/repo/blob/b2/b2_storage_test.go @@ -21,6 +21,8 @@ ) func getEnvOrSkip(t *testing.T, name string) string { + t.Helper() + value := os.Getenv(name) if value == "" { t.Skip(fmt.Sprintf("%s not provided", name)) diff --git a/repo/blob/filesystem/filesystem_storage_test.go b/repo/blob/filesystem/filesystem_storage_test.go index fcbb98846..0946c364c 100644 --- a/repo/blob/filesystem/filesystem_storage_test.go +++ b/repo/blob/filesystem/filesystem_storage_test.go @@ -124,6 +124,8 @@ func TestFileStorageConcurrency(t *testing.T) { } func verifyBlobTimestampOrder(t *testing.T, st blob.Storage, want ...blob.ID) { + t.Helper() + blobs, err := blob.ListAllBlobs(testlogging.Context(t), st, "") if err != nil { t.Errorf("error listing blobs: %v", err) diff --git a/repo/blob/gcs/gcs_storage.go b/repo/blob/gcs/gcs_storage.go index a48bbe05a..c132623d7 100644 --- a/repo/blob/gcs/gcs_storage.go +++ b/repo/blob/gcs/gcs_storage.go @@ -6,6 +6,7 @@ "encoding/json" "fmt" "io/ioutil" + "net/http" "time" gcsclient "cloud.google.com/go/storage" @@ -97,7 +98,7 @@ func exponentialBackoff(ctx context.Context, desc string, att retry.AttemptFunc) func isRetriableError(err error) bool { var apiError *googleapi.Error if errors.As(err, &apiError) { - return apiError.Code >= 500 + return apiError.Code >= http.StatusInternalServerError } switch { diff --git a/repo/blob/s3/s3_storage.go b/repo/blob/s3/s3_storage.go index f513f42be..00dd74289 100644 --- a/repo/blob/s3/s3_storage.go +++ b/repo/blob/s3/s3_storage.go @@ -91,7 +91,7 @@ func isRetriableError(err error) bool { if errors.As(err, &me) { // retry on server errors, not on client errors - return me.StatusCode >= 500 + return me.StatusCode >= http.StatusInternalServerError } if strings.Contains(strings.ToLower(err.Error()), "http") { diff --git a/repo/blob/s3/s3_storage_test.go b/repo/blob/s3/s3_storage_test.go index b7e348f31..a12373407 100644 --- a/repo/blob/s3/s3_storage_test.go +++ b/repo/blob/s3/s3_storage_test.go @@ -249,10 +249,10 @@ func testStorage(t *testutil.RetriableT, options *Options) { } func TestCustomTransportNoSSLVerify(t *testing.T) { - testURL(expiredBadSSL, t) - testURL(selfSignedBadSSL, t) - testURL(untrustedRootBadSSL, t) - testURL(wrongHostBadSSL, t) + testURL(t, expiredBadSSL) + testURL(t, selfSignedBadSSL) + testURL(t, untrustedRootBadSSL) + testURL(t, wrongHostBadSSL) } func getURL(url string, insecureSkipVerify bool) error { @@ -268,7 +268,9 @@ func getURL(url string, insecureSkipVerify bool) error { return nil } -func testURL(url string, t *testing.T) { +func testURL(t *testing.T, url string) { + t.Helper() + err := getURL(url, true) if err != nil { t.Fatalf("could not get url:%s, error:%v", url, err) diff --git a/repo/blob/sftp/sftp_storage.go b/repo/blob/sftp/sftp_storage.go index c7b82433c..9c3eba877 100644 --- a/repo/blob/sftp/sftp_storage.go +++ b/repo/blob/sftp/sftp_storage.go @@ -131,7 +131,7 @@ func (s *sftpImpl) PutBlobInPath(ctx context.Context, dirPath, fullPath string, err = s.cli.PosixRename(tempFile, fullPath) if err != nil { if removeErr := s.cli.Remove(tempFile); removeErr != nil { - fmt.Printf("warning: can't remove temp file: %v", removeErr) + log(ctx).Warningf("warning: can't remove temp file: %v", removeErr) } return errors.Wrap(err, "unexpected error renaming file on SFTP") diff --git a/repo/blob/sftp/sftp_storage_test.go b/repo/blob/sftp/sftp_storage_test.go index c69b2544b..c8f27a42b 100644 --- a/repo/blob/sftp/sftp_storage_test.go +++ b/repo/blob/sftp/sftp_storage_test.go @@ -41,6 +41,8 @@ func TestSFTPStorageValid(t *testing.T) { } func deleteBlobs(ctx context.Context, t *testing.T, st blob.Storage) { + t.Helper() + if err := st.ListBlobs(ctx, "", func(bm blob.Metadata) error { return st.DeleteBlob(ctx, bm.BlobID) }); err != nil { @@ -49,6 +51,8 @@ func deleteBlobs(ctx context.Context, t *testing.T, st blob.Storage) { } func createSFTPStorage(ctx context.Context, t *testing.T, embed bool) (blob.Storage, error) { + t.Helper() + host := os.Getenv("KOPIA_SFTP_TEST_HOST") if host == "" { t.Skip("KOPIA_SFTP_TEST_HOST not provided") @@ -105,6 +109,8 @@ func createSFTPStorage(ctx context.Context, t *testing.T, embed bool) (blob.Stor } func mustReadFileToString(t *testing.T, fname string) string { + t.Helper() + data, err := ioutil.ReadFile(fname) if err != nil { t.Fatal(err) diff --git a/repo/blob/webdav/webdav_storage_test.go b/repo/blob/webdav/webdav_storage_test.go index 305232a9c..9a111a7f0 100644 --- a/repo/blob/webdav/webdav_storage_test.go +++ b/repo/blob/webdav/webdav_storage_test.go @@ -88,6 +88,8 @@ func TestWebDAVStorageBuiltInServer(t *testing.T) { } func verifyWebDAVStorage(t *testing.T, url, username, password string, shardSpec []int) { + t.Helper() + ctx := testlogging.Context(t) st, err := New(testlogging.Context(t), &Options{ diff --git a/repo/compression/compressor_s2.go b/repo/compression/compressor_s2.go index 666ee7db2..89737f00f 100644 --- a/repo/compression/compressor_s2.go +++ b/repo/compression/compressor_s2.go @@ -10,11 +10,16 @@ "github.com/kopia/kopia/internal/iocopy" ) +const ( + s2Parallel4Concurrency = 4 + s2Parallel8Concurrency = 8 +) + func init() { RegisterCompressor("s2-default", newS2Compressor(headerS2Default)) RegisterCompressor("s2-better", newS2Compressor(headerS2Better, s2.WriterBetterCompression())) - RegisterCompressor("s2-parallel-4", newS2Compressor(headerS2Parallel4, s2.WriterConcurrency(4))) //nolint:gomnd - RegisterCompressor("s2-parallel-8", newS2Compressor(headerS2Parallel8, s2.WriterConcurrency(8))) //nolint:gomnd + RegisterCompressor("s2-parallel-4", newS2Compressor(headerS2Parallel4, s2.WriterConcurrency(s2Parallel4Concurrency))) + RegisterCompressor("s2-parallel-8", newS2Compressor(headerS2Parallel8, s2.WriterConcurrency(s2Parallel8Concurrency))) } func newS2Compressor(id HeaderID, opts ...s2.WriterOption) Compressor { diff --git a/repo/content/blob_crypto.go b/repo/content/blob_crypto.go index 880bb1c5c..d30999457 100644 --- a/repo/content/blob_crypto.go +++ b/repo/content/blob_crypto.go @@ -22,7 +22,7 @@ func getIndexBlobIV(s blob.ID) ([]byte, error) { return nil, errors.Errorf("blob id too short: %v", s) } - return hex.DecodeString(string(s[len(s)-(aes.BlockSize*2):])) + return hex.DecodeString(string(s[len(s)-(aes.BlockSize*2):])) //nolint:gomnd } func encryptFullBlob(h hashing.HashFunc, enc encryption.Encryptor, data []byte, prefix blob.ID, sessionID SessionID) (blob.ID, []byte, error) { diff --git a/repo/content/builder.go b/repo/content/builder.go index 2767b03f1..ce2bf7223 100644 --- a/repo/content/builder.go +++ b/repo/content/builder.go @@ -40,7 +40,6 @@ func (b packIndexBuilder) clone() packIndexBuilder { } // Add adds a new entry to the builder or conditionally replaces it if the timestamp is greater. -// nolint:gocritic func (b packIndexBuilder) Add(i Info) { old, ok := b[i.ID] if !ok || i.TimestampSeconds >= old.TimestampSeconds { diff --git a/repo/content/content_cache_test.go b/repo/content/content_cache_test.go index f99ae5dff..bc61480fc 100644 --- a/repo/content/content_cache_test.go +++ b/repo/content/content_cache_test.go @@ -21,6 +21,8 @@ ) func newUnderlyingStorageForContentCacheTesting(t *testing.T) blob.Storage { + t.Helper() + ctx := testlogging.Context(t) data := blobtesting.DataMap{} st := blobtesting.NewMapStorage(data, nil, nil) @@ -129,6 +131,8 @@ func TestDiskContentCache(t *testing.T) { } func verifyContentCache(t *testing.T, cache contentCache) { + t.Helper() + ctx := testlogging.Context(t) t.Run("GetContentContent", func(t *testing.T) { diff --git a/repo/content/content_formatter_test.go b/repo/content/content_formatter_test.go index 14496ccb8..1e2003973 100644 --- a/repo/content/content_formatter_test.go +++ b/repo/content/content_formatter_test.go @@ -92,6 +92,8 @@ func TestFormatters(t *testing.T) { } func verifyEndToEndFormatter(ctx context.Context, t *testing.T, hashAlgo, encryptionAlgo string) { + t.Helper() + data := blobtesting.DataMap{} keyTime := map[blob.ID]time.Time{} st := blobtesting.NewMapStorage(data, keyTime, nil) diff --git a/repo/content/content_manager.go b/repo/content/content_manager.go index 231f0dfc6..2dec4b2b7 100644 --- a/repo/content/content_manager.go +++ b/repo/content/content_manager.go @@ -142,7 +142,6 @@ func (bm *WriteManager) DeleteContent(ctx context.Context, contentID ID) error { } // Intentionally passing bi by value. -// nolint:gocritic func (bm *WriteManager) deletePreexistingContent(ctx context.Context, ci Info) error { if ci.Deleted { return nil diff --git a/repo/content/content_manager_lock_free.go b/repo/content/content_manager_lock_free.go index 652b9cd1f..93073f4f3 100644 --- a/repo/content/content_manager_lock_free.go +++ b/repo/content/content_manager_lock_free.go @@ -127,7 +127,7 @@ func (bm *WriteManager) preparePackDataContent(ctx context.Context, pp *pendingP } func getPackedContentIV(output []byte, contentID ID) ([]byte, error) { - n, err := hex.Decode(output, []byte(contentID[len(contentID)-(aes.BlockSize*2):])) + n, err := hex.Decode(output, []byte(contentID[len(contentID)-(aes.BlockSize*2):])) //nolint:gomnd if err != nil { return nil, errors.Wrapf(err, "error decoding content IV from %v", contentID) } diff --git a/repo/content/content_manager_own_writes.go b/repo/content/content_manager_own_writes.go index e45d81967..2f66def29 100644 --- a/repo/content/content_manager_own_writes.go +++ b/repo/content/content_manager_own_writes.go @@ -25,8 +25,7 @@ type ownWritesCache interface { } // nullOwnWritesCache is an implementation of ownWritesCache that ignores all changes. -type nullOwnWritesCache struct { -} +type nullOwnWritesCache struct{} func (n *nullOwnWritesCache) add(ctx context.Context, mb blob.Metadata) error { return nil diff --git a/repo/content/content_manager_test.go b/repo/content/content_manager_test.go index b7b7d991f..d2619b56f 100644 --- a/repo/content/content_manager_test.go +++ b/repo/content/content_manager_test.go @@ -1144,6 +1144,8 @@ func TestFlushWaitsForAllPendingWriters(t *testing.T) { } func verifyAllDataPresent(ctx context.Context, t *testing.T, data map[blob.ID][]byte, contentIDs map[ID]bool) { + t.Helper() + bm := newTestContentManager(t, data, nil, nil) defer bm.Close(ctx) _ = bm.IterateContents(ctx, IterateOptions{}, func(ci Info) error { @@ -1771,6 +1773,8 @@ func TestVersionCompatibility(t *testing.T) { } func verifyVersionCompat(t *testing.T, writeVersion int) { + t.Helper() + ctx := testlogging.Context(t) // create content manager that writes 'writeVersion' and reads all versions >= minSupportedReadVersion @@ -1882,6 +1886,8 @@ func TestReadsOwnWritesWithEventualConsistencyInMemoryOwnWritesCache(t *testing. } func verifyReadsOwnWrites(t *testing.T, st blob.Storage, timeNow func() time.Time, sharedOwnWritesCache ownWritesCache) { + t.Helper() + ctx := testlogging.Context(t) cachingOptions := &CachingOptions{} @@ -1916,6 +1922,8 @@ func verifyReadsOwnWrites(t *testing.T, st blob.Storage, timeNow func() time.Tim } func verifyContentManagerDataSet(ctx context.Context, t *testing.T, mgr *WriteManager, dataSet map[ID][]byte) { + t.Helper() + for contentID, originalPayload := range dataSet { v, err := mgr.GetContent(ctx, contentID) if err != nil { @@ -1930,15 +1938,22 @@ func verifyContentManagerDataSet(ctx context.Context, t *testing.T, mgr *WriteMa } func newTestContentManager(t *testing.T, data blobtesting.DataMap, keyTime map[blob.ID]time.Time, timeFunc func() time.Time) *WriteManager { + t.Helper() + st := blobtesting.NewMapStorage(data, keyTime, timeFunc) + return newTestContentManagerWithStorage(t, st, timeFunc) } func newTestContentManagerWithStorage(t *testing.T, st blob.Storage, timeFunc func() time.Time) *WriteManager { + t.Helper() + return newTestContentManagerWithStorageAndCaching(t, st, nil, timeFunc) } func newTestContentManagerWithStorageAndOptions(t *testing.T, st blob.Storage, co *CachingOptions, opts *ManagerOptions) *WriteManager { + t.Helper() + bm, err := NewManager(testlogging.Context(t), st, &FormattingOptions{ Hash: "HMAC-SHA256", Encryption: "AES256-GCM-HMAC-SHA256", @@ -1956,6 +1971,8 @@ func newTestContentManagerWithStorageAndOptions(t *testing.T, st blob.Storage, c } func newTestContentManagerWithStorageAndCaching(t *testing.T, st blob.Storage, co *CachingOptions, timeFunc func() time.Time) *WriteManager { + t.Helper() + if timeFunc == nil { timeFunc = faketime.AutoAdvance(fakeTime, 1*time.Second) } diff --git a/repo/content/format.go b/repo/content/format.go index 64e806a20..b232f70d1 100644 --- a/repo/content/format.go +++ b/repo/content/format.go @@ -2,10 +2,16 @@ import ( "encoding/binary" + "math" "github.com/pkg/errors" ) +const ( + timestampShift = 16 + packedFormatVersionShift = 8 +) + // Format describes a format of a single pack index. The actual structure is not used, // it's purely for documentation purposes. // The struct is byte-aligned. @@ -52,11 +58,11 @@ func (e *entry) IsDeleted() bool { } func (e *entry) TimestampSeconds() int64 { - return int64(e.timestampAndFlags >> 16) // nolint:gomnd + return int64(e.timestampAndFlags >> timestampShift) } func (e *entry) PackedFormatVersion() byte { - return byte(e.timestampAndFlags >> 8) // nolint:gomnd + return byte(e.timestampAndFlags >> packedFormatVersionShift) } func (e *entry) PackFileLength() byte { @@ -68,7 +74,7 @@ func (e *entry) PackFileOffset() uint32 { } func (e *entry) PackedOffset() uint32 { - return e.packedOffset & 0x7fffffff + return e.packedOffset & math.MaxInt32 } func (e *entry) PackedLength() uint32 { diff --git a/repo/content/index_blob_manager_test.go b/repo/content/index_blob_manager_test.go index 04c4e26b0..b8c3c621a 100644 --- a/repo/content/index_blob_manager_test.go +++ b/repo/content/index_blob_manager_test.go @@ -345,6 +345,8 @@ func randomDuration(max time.Duration) time.Duration { } func verifyIndexBlobManagerPreventsResurrectOfDeletedContents(t *testing.T, delay1, delay2, delay3, delay4, delay5 time.Duration) { + t.Helper() + t.Logf("delays: %v %v %v %v %v", delay1, delay2, delay3, delay4, delay5) storageData := blobtesting.DataMap{} @@ -691,6 +693,8 @@ func keysWithPrefix(data blobtesting.DataMap, prefix blob.ID) []blob.ID { } func mustRegisterCompaction(t *testing.T, m indexBlobManager, inputs, outputs []blob.Metadata) { + t.Helper() + t.Logf("compacting %v to %v", inputs, outputs) err := m.registerCompaction(testlogging.Context(t), inputs, outputs) @@ -700,6 +704,8 @@ func mustRegisterCompaction(t *testing.T, m indexBlobManager, inputs, outputs [] } func mustWriteIndexBlob(t *testing.T, m indexBlobManager, data string) blob.Metadata { + t.Helper() + t.Logf("writing index blob %q", data) blobMD, err := m.writeIndexBlob(testlogging.Context(t), []byte(data), "") @@ -734,6 +740,8 @@ func assertIndexBlobList(t *testing.T, m indexBlobManager, wantMD ...blob.Metada } func newIndexBlobManagerForTesting(t *testing.T, st blob.Storage, localTimeNow func() time.Time) indexBlobManager { + t.Helper() + p := &FormattingOptions{ Encryption: encryption.DeprecatedNoneAlgorithm, Hash: hashing.DefaultAlgorithm, diff --git a/repo/content/merged_test.go b/repo/content/merged_test.go index 1e26cfd90..4c2b8eed1 100644 --- a/repo/content/merged_test.go +++ b/repo/content/merged_test.go @@ -134,6 +134,8 @@ func TestMerged(t *testing.T) { } func iterateIDRange(t *testing.T, m packIndex, r IDRange) []ID { + t.Helper() + var inOrder []ID assertNoError(t, m.Iterate(r, func(i Info) error { diff --git a/repo/encryption/deprecated_ctr_encryptor.go b/repo/encryption/deprecated_ctr_encryptor.go index a412b4d51..a2afef268 100644 --- a/repo/encryption/deprecated_ctr_encryptor.go +++ b/repo/encryption/deprecated_ctr_encryptor.go @@ -7,6 +7,12 @@ "github.com/pkg/errors" ) +const ( + aes128KeyLength = 16 + aes192KeyLength = 24 + aes256KeyLength = 32 +) + // ctrEncryptor implements encrypted format which uses CTR mode of a content cipher with nonce==IV. type ctrEncryptor struct { createCipher func() (cipher.Block, error) @@ -79,7 +85,7 @@ func newCTREncryptorFactory(keySize int, createCipherWithKey func(key []byte) (c } func init() { - Register("AES-128-CTR", "DEPRECATED: AES-128 in CTR mode", true, newCTREncryptorFactory(16, aes.NewCipher)) //nolint:gomnd - Register("AES-192-CTR", "DEPRECATED: AES-192 in CTR mode", true, newCTREncryptorFactory(24, aes.NewCipher)) //nolint:gomnd - Register("AES-256-CTR", "DEPRECATED: AES-256 in CTR mode", true, newCTREncryptorFactory(32, aes.NewCipher)) //nolint:gomnd + Register("AES-128-CTR", "DEPRECATED: AES-128 in CTR mode", true, newCTREncryptorFactory(aes128KeyLength, aes.NewCipher)) + Register("AES-192-CTR", "DEPRECATED: AES-192 in CTR mode", true, newCTREncryptorFactory(aes192KeyLength, aes.NewCipher)) + Register("AES-256-CTR", "DEPRECATED: AES-256 in CTR mode", true, newCTREncryptorFactory(aes256KeyLength, aes.NewCipher)) } diff --git a/repo/encryption/encryption_test.go b/repo/encryption/encryption_test.go index a5505cbf5..2b661a236 100644 --- a/repo/encryption/encryption_test.go +++ b/repo/encryption/encryption_test.go @@ -177,6 +177,8 @@ func TestCiphertextSamples(t *testing.T) { } func verifyCiphertextSamples(t *testing.T, masterKey, contentID, payload []byte, samples map[string]string) { + t.Helper() + for _, encryptionAlgo := range encryption.SupportedAlgorithms(true) { enc, err := encryption.CreateEncryptor(parameters{encryptionAlgo, masterKey}) if err != nil { diff --git a/repo/encryption/null_encryptor.go b/repo/encryption/null_encryptor.go index 7dbc56443..84173e4bf 100644 --- a/repo/encryption/null_encryptor.go +++ b/repo/encryption/null_encryptor.go @@ -1,8 +1,7 @@ package encryption // nullEncryptor implements non-encrypted format. -type nullEncryptor struct { -} +type nullEncryptor struct{} func (fi nullEncryptor) Encrypt(output, plainText, contentID []byte) ([]byte, error) { return append(output, plainText...), nil diff --git a/repo/manifest/manifest_manager_test.go b/repo/manifest/manifest_manager_test.go index 9fad78a57..359d8bd2e 100644 --- a/repo/manifest/manifest_manager_test.go +++ b/repo/manifest/manifest_manager_test.go @@ -292,6 +292,8 @@ func sortIDs(s []ID) { } func newManagerForTesting(ctx context.Context, t *testing.T, data blobtesting.DataMap) *Manager { + t.Helper() + st := blobtesting.NewMapStorage(data, nil, nil) bm, err := content.NewManager(ctx, st, &content.FormattingOptions{ diff --git a/repo/object/object_manager_test.go b/repo/object/object_manager_test.go index 3ebb06862..ef7f25c88 100644 --- a/repo/object/object_manager_test.go +++ b/repo/object/object_manager_test.go @@ -72,10 +72,14 @@ func (f *fakeContentManager) Flush(ctx context.Context) error { } func setupTest(t *testing.T) (map[content.ID][]byte, *Manager) { + t.Helper() + return setupTestWithData(t, map[content.ID][]byte{}) } func setupTestWithData(t *testing.T, data map[content.ID][]byte) (map[content.ID][]byte, *Manager) { + t.Helper() + r, err := NewObjectManager(testlogging.Context(t), &fakeContentManager{data: data}, Format{ Splitter: "FIXED-1M", }) @@ -294,6 +298,8 @@ func verifyNoError(t *testing.T, err error) { } func verifyIndirectBlock(ctx context.Context, t *testing.T, om *Manager, oid ID) { + t.Helper() + for indexContentID, isIndirect := oid.IndexObjectID(); isIndirect; indexContentID, isIndirect = indexContentID.IndexObjectID() { if c, _, ok := indexContentID.ContentID(); ok { if !c.HasPrefix() { diff --git a/repo/repository_test.go b/repo/repository_test.go index 4e2aaf36b..0b060bb19 100644 --- a/repo/repository_test.go +++ b/repo/repository_test.go @@ -198,6 +198,8 @@ func TestReaderStoredBlockNotFound(t *testing.T) { } func writeObject(ctx context.Context, t *testing.T, rep repo.Repository, data []byte, testCaseID string) object.ID { + t.Helper() + w := rep.NewObjectWriter(ctx, object.WriterOptions{}) if _, err := w.Write(data); err != nil { t.Fatalf("can't write object %q - write failed: %v", testCaseID, err) diff --git a/repo/splitter/splitter.go b/repo/splitter/splitter.go index d8cc63307..6005ddc46 100644 --- a/repo/splitter/splitter.go +++ b/repo/splitter/splitter.go @@ -63,7 +63,7 @@ func SupportedAlgorithms() []string { } func megabytes(mb int) int { - return mb << 20 + return mb << 20 //nolint:gomnd } // GetFactory gets splitter factory with a specified name or nil if not found. diff --git a/snapshot/policy/actions_policy.go b/snapshot/policy/actions_policy.go index f06c48fb1..141066561 100644 --- a/snapshot/policy/actions_policy.go +++ b/snapshot/policy/actions_policy.go @@ -25,7 +25,6 @@ type ActionCommand struct { } // Merge applies default values from the provided policy. -// nolint:gocritic func (p *ActionsPolicy) Merge(src ActionsPolicy) { if p.BeforeSnapshotRoot == nil { p.BeforeSnapshotRoot = src.BeforeSnapshotRoot diff --git a/snapshot/policy/policy_manager_test.go b/snapshot/policy/policy_manager_test.go index f89bbeb15..7ca68fbb5 100644 --- a/snapshot/policy/policy_manager_test.go +++ b/snapshot/policy/policy_manager_test.go @@ -139,6 +139,8 @@ func TestPolicyManagerInheritanceTest(t *testing.T) { } func clonePolicy(t *testing.T, p *Policy) *Policy { + t.Helper() + j, err := json.Marshal(p) if err != nil { t.Fatalf("unable to marshal JSON: %v", err.Error()) @@ -158,6 +160,8 @@ func policyWithLabels(p *Policy, l map[string]string) *Policy { } func defaultPolicyWithKeepDaily(t *testing.T, keepDaily int) *Policy { + t.Helper() + p := clonePolicy(t, DefaultPolicy) p.RetentionPolicy.KeepDaily = &keepDaily diff --git a/snapshot/snapshot_test.go b/snapshot/snapshot_test.go index 807651e45..bf8e9de1f 100644 --- a/snapshot/snapshot_test.go +++ b/snapshot/snapshot_test.go @@ -115,6 +115,8 @@ func mustSaveSnapshot(t *testing.T, rep repo.Writer, man *snapshot.Manifest) man } func verifySources(t *testing.T, rep repo.Reader, sources ...snapshot.SourceInfo) { + t.Helper() + actualSources, err := snapshot.ListSources(testlogging.Context(t), rep) if err != nil { t.Errorf("error listing sources: %v", err) @@ -148,6 +150,8 @@ func verifyListSnapshots(t *testing.T, rep repo.Reader, src snapshot.SourceInfo, } func verifyLoadSnapshots(t *testing.T, rep repo.Reader, ids []manifest.ID, expected []*snapshot.Manifest) { + t.Helper() + got, err := snapshot.LoadSnapshots(testlogging.Context(t), rep, ids) if err != nil { t.Errorf("error loading manifests: %v", err) @@ -185,6 +189,8 @@ func sourcesToStrings(sources ...snapshot.SourceInfo) []string { } func mustAbs(t *testing.T, p string) string { + t.Helper() + p2, err := filepath.Abs(p) if err != nil { t.Fatal(err) diff --git a/snapshot/snapshotfs/all_sources.go b/snapshot/snapshotfs/all_sources.go index fc8f660dd..a5e69bb21 100644 --- a/snapshot/snapshotfs/all_sources.go +++ b/snapshot/snapshotfs/all_sources.go @@ -30,7 +30,7 @@ func (s *repositoryAllSources) ModTime() time.Time { } func (s *repositoryAllSources) Mode() os.FileMode { - return 0o555 | os.ModeDir + return 0o555 | os.ModeDir // nolint:gomnd } func (s *repositoryAllSources) Size() int64 { diff --git a/snapshot/snapshotfs/source_directories.go b/snapshot/snapshotfs/source_directories.go index d98607df1..fccd3ea13 100644 --- a/snapshot/snapshotfs/source_directories.go +++ b/snapshot/snapshotfs/source_directories.go @@ -26,7 +26,7 @@ func (s *sourceDirectories) Name() string { } func (s *sourceDirectories) Mode() os.FileMode { - return 0o555 | os.ModeDir + return 0o555 | os.ModeDir // nolint:gomnd } func (s *sourceDirectories) ModTime() time.Time { diff --git a/snapshot/snapshotfs/source_snapshots.go b/snapshot/snapshotfs/source_snapshots.go index a7faa0a70..4d206b61b 100644 --- a/snapshot/snapshotfs/source_snapshots.go +++ b/snapshot/snapshotfs/source_snapshots.go @@ -28,7 +28,7 @@ func (s *sourceSnapshots) Name() string { } func (s *sourceSnapshots) Mode() os.FileMode { - return 0o555 | os.ModeDir + return 0o555 | os.ModeDir // nolint:gomnd } func (s *sourceSnapshots) Size() int64 { diff --git a/snapshot/snapshotfs/upload_progress.go b/snapshot/snapshotfs/upload_progress.go index 0c383a17b..7334663d5 100644 --- a/snapshot/snapshotfs/upload_progress.go +++ b/snapshot/snapshotfs/upload_progress.go @@ -42,8 +42,7 @@ type UploadProgress interface { } // NullUploadProgress is an implementation of UploadProgress that does not produce any output. -type NullUploadProgress struct { -} +type NullUploadProgress struct{} // UploadStarted implements UploadProgress. func (p *NullUploadProgress) UploadStarted() {} diff --git a/snapshot/snapshotgc/gc.go b/snapshot/snapshotgc/gc.go index 04602da98..490992bd9 100644 --- a/snapshot/snapshotgc/gc.go +++ b/snapshot/snapshotgc/gc.go @@ -74,7 +74,6 @@ func findInUseContentIDs(ctx context.Context, rep repo.Reader, used *sync.Map) e } // Run performs garbage collection on all the snapshots in the repository. -// nolint:gocognit func Run(ctx context.Context, rep *repo.DirectRepository, params maintenance.SnapshotGCParams, gcDelete bool) (Stats, error) { var st Stats diff --git a/snapshot/snapshotmaintenance/snapshotmaintenance_test.go b/snapshot/snapshotmaintenance/snapshotmaintenance_test.go index 8ac652702..7d36696d8 100644 --- a/snapshot/snapshotmaintenance/snapshotmaintenance_test.go +++ b/snapshot/snapshotmaintenance/snapshotmaintenance_test.go @@ -186,6 +186,8 @@ func (th *testHarness) fakeTimeOpenRepoOption(o *repo.Options) { } func (th *testHarness) openAnother(t *testing.T) repo.Repository { + t.Helper() + r := th.MustConnectOpenAnother(t, th.fakeTimeOpenRepoOption) t.Cleanup(func() { diff --git a/tests/end_to_end_test/server_start_test.go b/tests/end_to_end_test/server_start_test.go index 712fea171..f020c042d 100644 --- a/tests/end_to_end_test/server_start_test.go +++ b/tests/end_to_end_test/server_start_test.go @@ -423,6 +423,8 @@ func verifySourceCount(t *testing.T, cli *apiclient.KopiaAPIClient, match *snaps } func verifyUIServedWithCorrectTitle(t *testing.T, cli *apiclient.KopiaAPIClient, sp serverParameters) { + t.Helper() + req, err := http.NewRequestWithContext(context.Background(), "GET", sp.baseURL, nil) if err != nil { t.Fatalf("unable to create HTTP request: %v", err) diff --git a/tests/end_to_end_test/snapshot_actions_test.go b/tests/end_to_end_test/snapshot_actions_test.go index 37f3278b1..aa14e8d57 100644 --- a/tests/end_to_end_test/snapshot_actions_test.go +++ b/tests/end_to_end_test/snapshot_actions_test.go @@ -321,6 +321,8 @@ func TestSnapshotActionsEnable(t *testing.T) { } func tmpfileWithContents(t *testing.T, contents string) string { + t.Helper() + f, err := ioutil.TempFile("", "kopia-test") verifyNoError(t, err) @@ -350,6 +352,8 @@ func verifyNoError(t *testing.T, err error) { } func mustReadEnvFile(t *testing.T, fname string) map[string]string { + t.Helper() + f, err := os.Open(fname) verifyNoError(t, err) diff --git a/tests/end_to_end_test/snapshot_delete_test.go b/tests/end_to_end_test/snapshot_delete_test.go index 80ed5b7a6..aebad2a40 100644 --- a/tests/end_to_end_test/snapshot_delete_test.go +++ b/tests/end_to_end_test/snapshot_delete_test.go @@ -102,6 +102,8 @@ func(manifestID, objectID string, source testenv.SourceInfo) []string { } func testSnapshotDelete(t *testing.T, argMaker deleteArgMaker, expectDeleteSucceeds bool) { + t.Helper() + e := testenv.NewCLITest(t) defer e.RunAndExpectSuccess(t, "repo", "disconnect") @@ -232,6 +234,8 @@ func TestSnapshotDeleteRestore(t *testing.T) { } func assertEmptyDir(t *testing.T, dir string) { + t.Helper() + // Make sure the restore did not happen from the deleted snapshot fileInfo, err := ioutil.ReadDir(dir) testenv.AssertNoError(t, err) diff --git a/tests/end_to_end_test/snapshot_fail_test.go b/tests/end_to_end_test/snapshot_fail_test.go index 86f0f1d02..ec25b1a3d 100644 --- a/tests/end_to_end_test/snapshot_fail_test.go +++ b/tests/end_to_end_test/snapshot_fail_test.go @@ -207,7 +207,7 @@ func TestSnapshotFail(t *testing.T) { e.RunAndExpectSuccess(t, "policy", "set", snapSource, "--ignore-dir-errors", tcIgnoreDirErr, "--ignore-file-errors", tcIgnoreFileErr) restoreDir := fmt.Sprintf("%s%d_%v_%v", restoreDirPrefix, tcIdx, tcIgnoreDirErr, tcIgnoreFileErr) - testPermissions(e, t, snapSource, modifyEntry, restoreDir, tc.expectSuccess) + testPermissions(t, e, snapSource, modifyEntry, restoreDir, tc.expectSuccess) e.RunAndExpectSuccess(t, "policy", "remove", snapSource) }) @@ -217,6 +217,8 @@ func TestSnapshotFail(t *testing.T) { } func createSimplestFileTree(t *testing.T, dirDepth, currDepth int, currPath string) { + t.Helper() + dirname := fmt.Sprintf("dir%d", currDepth) dirPath := filepath.Join(currPath, dirname) err := os.MkdirAll(dirPath, 0o700) @@ -243,7 +245,7 @@ func createSimplestFileTree(t *testing.T, dirDepth, currDepth int, currPath stri // files and directories (if present). It issues the kopia snapshot command // against "source" and will test permissions against all entries in "parentDir". // It returns the number of successful snapshot operations. -func testPermissions(e *testenv.CLITest, t *testing.T, source, modifyEntry, restoreDir string, expectSuccess map[os.FileMode]bool) int { +func testPermissions(t *testing.T, e *testenv.CLITest, source, modifyEntry, restoreDir string, expectSuccess map[os.FileMode]bool) int { t.Helper() var numSuccessfulSnapshots int @@ -283,6 +285,8 @@ func testPermissions(e *testenv.CLITest, t *testing.T, source, modifyEntry, rest } func parseSnapID(t *testing.T, lines []string) string { + t.Helper() + pattern := regexp.MustCompile(`Created snapshot with root \S+ and ID (\S+) in .*`) for _, l := range lines { diff --git a/tests/endurance_test/endurance_test.go b/tests/endurance_test/endurance_test.go index 2301e6ac5..2b1798abd 100644 --- a/tests/endurance_test/endurance_test.go +++ b/tests/endurance_test/endurance_test.go @@ -112,6 +112,8 @@ type runnerState struct { } func actionSnapshotExisting(t *testing.T, e *testenv.CLITest, s *runnerState) { + t.Helper() + randomPath := s.dirs[rand.Intn(len(s.dirs))] e.RunAndExpectSuccess(t, "snapshot", "create", randomPath, "--no-progress") @@ -119,6 +121,8 @@ func actionSnapshotExisting(t *testing.T, e *testenv.CLITest, s *runnerState) { } func actionSnapshotAll(t *testing.T, e *testenv.CLITest, s *runnerState) { + t.Helper() + if !s.snapshottedAnything { return } @@ -127,6 +131,8 @@ func actionSnapshotAll(t *testing.T, e *testenv.CLITest, s *runnerState) { } func actionSnapshotVerify(t *testing.T, e *testenv.CLITest, s *runnerState) { + t.Helper() + if !s.snapshottedAnything { return } @@ -135,6 +141,8 @@ func actionSnapshotVerify(t *testing.T, e *testenv.CLITest, s *runnerState) { } func actionContentVerify(t *testing.T, e *testenv.CLITest, s *runnerState) { + t.Helper() + if !s.snapshottedAnything { return } @@ -143,6 +151,8 @@ func actionContentVerify(t *testing.T, e *testenv.CLITest, s *runnerState) { } func actionAddNewSource(t *testing.T, e *testenv.CLITest, s *runnerState) { + t.Helper() + if len(s.dirs) >= maxSourcesPerEnduranceRunner { return } @@ -163,6 +173,8 @@ func actionAddNewSource(t *testing.T, e *testenv.CLITest, s *runnerState) { } func actionMutateDirectoryTree(t *testing.T, e *testenv.CLITest, s *runnerState) { + t.Helper() + randomPath := s.dirs[rand.Intn(len(s.dirs))] testenv.CreateDirectoryTree(randomPath, testenv.DirectoryTreeOptions{ @@ -192,6 +204,8 @@ func pickRandomEnduranceTestAction() action { } func enduranceRunner(t *testing.T, runnerID int, fakeTimeServer, webdavServer string) { + t.Helper() + e := testenv.NewCLITest(t) e.Environment = append(e.Environment, diff --git a/tests/repository_stress_test/repository_stress_test.go b/tests/repository_stress_test/repository_stress_test.go index e0f62298d..abb1fbfda 100644 --- a/tests/repository_stress_test/repository_stress_test.go +++ b/tests/repository_stress_test/repository_stress_test.go @@ -130,6 +130,8 @@ func TestStressRepository(t *testing.T) { } func longLivedRepositoryTest(ctx context.Context, t *testing.T, cancel chan struct{}, configFile string, wg *sync.WaitGroup) { + t.Helper() + defer wg.Done() rep, err := repo.Open(ctx, configFile, masterPassword, &repo.Options{}) @@ -155,6 +157,8 @@ func longLivedRepositoryTest(ctx context.Context, t *testing.T, cancel chan stru } func repositoryTest(ctx context.Context, t *testing.T, cancel chan struct{}, rep *repo.DirectRepository) { + t.Helper() + workTypes := []*struct { name string fun func(ctx context.Context, t *testing.T, r *repo.DirectRepository) error @@ -218,6 +222,8 @@ func repositoryTest(ctx context.Context, t *testing.T, cancel chan struct{}, rep } func writeRandomBlock(ctx context.Context, t *testing.T, r *repo.DirectRepository) error { + t.Helper() + data := make([]byte, 1000) cryptorand.Read(data) @@ -237,6 +243,8 @@ func writeRandomBlock(ctx context.Context, t *testing.T, r *repo.DirectRepositor } func readKnownBlock(ctx context.Context, t *testing.T, r *repo.DirectRepository) error { + t.Helper() + knownBlocksMutex.Lock() if len(knownBlocks) == 0 { knownBlocksMutex.Unlock() @@ -255,6 +263,8 @@ func readKnownBlock(ctx context.Context, t *testing.T, r *repo.DirectRepository) } func listContents(ctx context.Context, t *testing.T, r *repo.DirectRepository) error { + t.Helper() + return r.Content.IterateContents( ctx, content.IterateOptions{}, @@ -263,6 +273,8 @@ func(i content.Info) error { return nil }, } func listAndReadAllContents(ctx context.Context, t *testing.T, r *repo.DirectRepository) error { + t.Helper() + return r.Content.IterateContents( ctx, content.IterateOptions{}, @@ -282,18 +294,26 @@ func(ci content.Info) error { } func compact(ctx context.Context, t *testing.T, r *repo.DirectRepository) error { + t.Helper() + return r.Content.CompactIndexes(ctx, content.CompactOptions{MaxSmallBlobs: 1}) } func flush(ctx context.Context, t *testing.T, r *repo.DirectRepository) error { + t.Helper() + return r.Flush(ctx) } func refresh(ctx context.Context, t *testing.T, r *repo.DirectRepository) error { + t.Helper() + return r.Refresh(ctx) } func readRandomManifest(ctx context.Context, t *testing.T, r *repo.DirectRepository) error { + t.Helper() + manifests, err := r.FindManifests(ctx, nil) if err != nil { return err @@ -311,6 +331,8 @@ func readRandomManifest(ctx context.Context, t *testing.T, r *repo.DirectReposit } func writeRandomManifest(ctx context.Context, t *testing.T, r *repo.DirectRepository) error { + t.Helper() + key1 := fmt.Sprintf("key-%v", rand.Intn(10)) key2 := fmt.Sprintf("key-%v", rand.Intn(10)) val1 := fmt.Sprintf("val1-%v", rand.Intn(10)) diff --git a/tests/robustness/engine/action.go b/tests/robustness/engine/action.go index 92e12c27e..821dffba9 100644 --- a/tests/robustness/engine/action.go +++ b/tests/robustness/engine/action.go @@ -248,7 +248,7 @@ type Action struct { f: func(e *Engine, opts map[string]string, l *LogEntry) (out map[string]string, err error) { // Directory depth maxDirDepth := getOptAsIntOrDefault(MaxDirDepthField, opts, defaultMaxDirDepth) - dirDepth := rand.Intn(maxDirDepth + 1) //nolint:gosec + dirDepth := rand.Intn(maxDirDepth + 1) // File size range maxFileSizeB := getOptAsIntOrDefault(MaxFileSizeField, opts, defaultMaxFileSize) diff --git a/tests/robustness/engine/engine_test.go b/tests/robustness/engine/engine_test.go index 4b99711cf..f90df92b4 100644 --- a/tests/robustness/engine/engine_test.go +++ b/tests/robustness/engine/engine_test.go @@ -91,6 +91,8 @@ func randomString(n int) string { } func makeTempS3Bucket(t *testing.T) (bucketName string, cleanupCB func()) { + t.Helper() + endpoint := "s3.amazonaws.com" accessKeyID := os.Getenv("AWS_ACCESS_KEY_ID") secretAccessKey := os.Getenv("AWS_SECRET_ACCESS_KEY") diff --git a/tests/stress_test/stress_test.go b/tests/stress_test/stress_test.go index 32329a6fe..57dd6bec9 100644 --- a/tests/stress_test/stress_test.go +++ b/tests/stress_test/stress_test.go @@ -35,6 +35,7 @@ func TestStressBlockManager(t *testing.T) { stressTestWithStorage(t, memst, duration) } +// nolint:thelper func stressTestWithStorage(t *testing.T, st blob.Storage, duration time.Duration) { ctx := testlogging.Context(t) @@ -65,6 +66,7 @@ func stressTestWithStorage(t *testing.T, st blob.Storage, duration time.Duration }) } +// nolint:thelper func stressWorker(ctx context.Context, t *testing.T, deadline time.Time, openMgr func() (*content.WriteManager, error), seed int64) { src := rand.NewSource(seed) rnd := rand.New(src) diff --git a/tests/testenv/cli_test_env.go b/tests/testenv/cli_test_env.go index 83a78d5b4..305f009a8 100644 --- a/tests/testenv/cli_test_env.go +++ b/tests/testenv/cli_test_env.go @@ -66,6 +66,8 @@ type SnapshotInfo struct { // NewCLITest creates a new instance of *CLITest. func NewCLITest(t *testing.T) *CLITest { + t.Helper() + exe := os.Getenv("KOPIA_EXE") if exe == "" { // exe = "kopia" @@ -276,7 +278,10 @@ func trimOutput(s string) string { // ListSnapshotsAndExpectSuccess lists given snapshots and parses the output. func (e *CLITest) ListSnapshotsAndExpectSuccess(t *testing.T, targets ...string) []SourceInfo { + t.Helper() + lines := e.RunAndExpectSuccess(t, append([]string{"snapshot", "list", "-l", "--manifest-id"}, targets...)...) + return mustParseSnapshots(t, lines) } @@ -288,13 +293,19 @@ type DirEntry struct { // ListDirectory lists a given directory and returns directory entries. func (e *CLITest) ListDirectory(t *testing.T, targets ...string) []DirEntry { + t.Helper() + lines := e.RunAndExpectSuccess(t, append([]string{"ls", "-l"}, targets...)...) + return mustParseDirectoryEntries(lines) } // ListDirectoryRecursive lists a given directory recursively and returns directory entries. func (e *CLITest) ListDirectoryRecursive(t *testing.T, targets ...string) []DirEntry { + t.Helper() + lines := e.RunAndExpectSuccess(t, append([]string{"ls", "-lr"}, targets...)...) + return mustParseDirectoryEntries(lines) } @@ -358,6 +369,8 @@ func CreateDirectoryTree(dirname string, options DirectoryTreeOptions, counters // MustCreateRandomFile creates a new file at the provided path with randomized contents. // It will fail with a test error if the creation does not succeed. func MustCreateRandomFile(t *testing.T, filePath string, options DirectoryTreeOptions, counters *DirectoryTreeCounters) { + t.Helper() + if err := CreateRandomFile(filePath, options, counters); err != nil { t.Fatal(err) } @@ -460,9 +473,12 @@ func createRandomSymlink(filename string, existingFiles []string, options Direct } func mustParseSnapshots(t *testing.T, lines []string) []SourceInfo { - var result []SourceInfo + t.Helper() - var currentSource *SourceInfo + var ( + result []SourceInfo + currentSource *SourceInfo + ) for _, l := range lines { if l == "" { @@ -503,6 +519,8 @@ func randomName(opt DirectoryTreeOptions) string { } func mustParseSnaphotInfo(t *testing.T, l string) SnapshotInfo { + t.Helper() + parts := strings.Split(l, " ") ts, err := time.Parse("2006-01-02 15:04:05 MST", strings.Join(parts[0:3], " ")) @@ -521,6 +539,8 @@ func mustParseSnaphotInfo(t *testing.T, l string) SnapshotInfo { } func mustParseSourceInfo(t *testing.T, l string) SourceInfo { + t.Helper() + p1 := strings.Index(l, "@") p2 := strings.Index(l, ":") diff --git a/tests/testenv/faketimeserver.go b/tests/testenv/faketimeserver.go index 0ff226210..eb27619a5 100644 --- a/tests/testenv/faketimeserver.go +++ b/tests/testenv/faketimeserver.go @@ -52,7 +52,7 @@ func (s *FakeTimeServer) ServeHTTP(w http.ResponseWriter, r *http.Request) { func NewFakeTimeServer(startTime time.Time, step time.Duration) *FakeTimeServer { return &FakeTimeServer{ nextTimeChunk: startTime, - timeChunkLength: 100 * step, + timeChunkLength: 100 * step, // nolint:gomnd step: step, } } diff --git a/tests/tools/fio/workload_test.go b/tests/tools/fio/workload_test.go index 6abb06e0d..60fa36368 100644 --- a/tests/tools/fio/workload_test.go +++ b/tests/tools/fio/workload_test.go @@ -93,6 +93,8 @@ func TestWriteFilesAtDepth(t *testing.T) { } func testWriteAtDepth(t *testing.T, r *Runner, depth, expFileCount int) { + t.Helper() + testSubdir := "test" testDirAbs := filepath.Join(r.LocalDataDir, testSubdir) @@ -189,6 +191,8 @@ func TestDeleteFilesAtDepth(t *testing.T) { } func testDeleteAtDepth(t *testing.T, r *Runner, wrDepth, delDepth, expDirCount int, expErr bool) { + t.Helper() + testSubdir := "test" testDirAbs := filepath.Join(r.LocalDataDir, testSubdir) @@ -243,6 +247,8 @@ func TestDeleteContentsAtDepth(t *testing.T) { { prob: 0.0, expFileCountChecker: func(t *testing.T, fileCount int) { + t.Helper() + if fileCount != 100 { t.Error("some files were deleted despite 0% probability") } @@ -251,6 +257,8 @@ func TestDeleteContentsAtDepth(t *testing.T) { { prob: 1.0, expFileCountChecker: func(t *testing.T, fileCount int) { + t.Helper() + if fileCount != 0 { t.Error("not all files were deleted despite 100% probability") } @@ -259,6 +267,8 @@ func TestDeleteContentsAtDepth(t *testing.T) { { prob: 0.5, expFileCountChecker: func(t *testing.T, fileCount int) { + t.Helper() + // Broad check: just make sure a 50% probability deleted something. // Extremely improbable that this causes a false failure; // akin to 100 coin flips all landing on the same side. @@ -272,6 +282,7 @@ func TestDeleteContentsAtDepth(t *testing.T) { } } +// nolint:thelper func testDeleteContentsAtDepth(t *testing.T, prob float32, checker func(t *testing.T, fileCount int)) { r, err := NewRunner() testenv.AssertNoError(t, err) diff --git a/tools/linter_version_test.go b/tools/linter_version_test.go new file mode 100644 index 000000000..64081d773 --- /dev/null +++ b/tools/linter_version_test.go @@ -0,0 +1,45 @@ +package tools_test + +import ( + "bufio" + "os" + "strings" + "testing" +) + +func TestLinterVersion(t *testing.T) { + makefileVersion := grepLine(t, "tools.mk", "GOLANGCI_LINT_VERSION=") + workflowVersion := grepLine(t, "../.github/workflows/lint.yml", "version: v") + + if !strings.HasPrefix(makefileVersion, workflowVersion+".") { + t.Fatalf("linter version out of sync (makefile %v, workflow %v)", makefileVersion, workflowVersion) + } +} + +// grepLine returns the contents of a line in the provided file that contains the provided prefix. +// the result will have the prefix and any whitespace removed. +func grepLine(t *testing.T, fname, prefix string) string { + t.Helper() + + f, err := os.Open(fname) + if err != nil { + t.Fatal(err) + } + defer f.Close() + + s := bufio.NewScanner(f) + for s.Scan() { + l := strings.TrimSpace(s.Text()) + if strings.HasPrefix(l, prefix) { + return strings.TrimPrefix(l, prefix) + } + } + + if s.Err() != nil { + t.Fatal(s.Err()) + } + + t.Fatalf("line starting with %v not found in %v", prefix, fname) + + return "" +} diff --git a/tools/tools.mk b/tools/tools.mk index 59e3d4837..e233395c8 100644 --- a/tools/tools.mk +++ b/tools/tools.mk @@ -134,7 +134,7 @@ SELF_DIR := $(subst /,$(slash),$(realpath $(dir $(lastword $(MAKEFILE_LIST))))) TOOLS_DIR:=$(SELF_DIR)$(slash).tools # tool versions -GOLANGCI_LINT_VERSION=1.33.0 +GOLANGCI_LINT_VERSION=1.35.2 NODE_VERSION=12.18.3 HUGO_VERSION=0.74.3 GOTESTSUM_VERSION=0.5.3