diff options
author | Jaime Martinez <jmartinez@gitlab.com> | 2022-02-24 00:22:52 +0300 |
---|---|---|
committer | Jaime Martinez <jmartinez@gitlab.com> | 2022-02-24 00:22:52 +0300 |
commit | 2bee7f9d4e4e3e87b3e5c682061c9b1db3218ea1 (patch) | |
tree | 693c51657cfa9dd29b0308202a3b9dbb3a0644e8 /internal | |
parent | 3ea4ea135e0d52b9717e2fd703f8e5136b95b868 (diff) | |
parent | b7d42ef5004b59d1885dcd332eaab64805d2578c (diff) |
Merge branch 'refactor/test-erroris' into 'master'
test: migrate to assertions using modern error checking
See merge request gitlab-org/gitlab-pages!684
Diffstat (limited to 'internal')
-rw-r--r-- | internal/config/validate_test.go | 7 | ||||
-rw-r--r-- | internal/httpfs/http_fs_test.go | 32 | ||||
-rw-r--r-- | internal/httprange/http_ranged_reader_test.go | 10 | ||||
-rw-r--r-- | internal/httptransport/transport_test.go | 3 | ||||
-rw-r--r-- | internal/redirects/redirects_test.go | 50 | ||||
-rw-r--r-- | internal/redirects/validations_test.go | 66 | ||||
-rw-r--r-- | internal/source/gitlab/cache/cache_test.go | 4 | ||||
-rw-r--r-- | internal/source/gitlab/client/client_test.go | 24 | ||||
-rw-r--r-- | internal/source/gitlab/factory_test.go | 2 | ||||
-rw-r--r-- | internal/vfs/local/root_test.go | 55 | ||||
-rw-r--r-- | internal/vfs/local/vfs_test.go | 19 | ||||
-rw-r--r-- | internal/vfs/zip/archive_test.go | 24 | ||||
-rw-r--r-- | internal/vfs/zip/vfs_test.go | 28 |
13 files changed, 127 insertions, 197 deletions
diff --git a/internal/config/validate_test.go b/internal/config/validate_test.go index 80e4ded3..6e96d7d3 100644 --- a/internal/config/validate_test.go +++ b/internal/config/validate_test.go @@ -1,7 +1,6 @@ package config import ( - "errors" "testing" "github.com/stretchr/testify/require" @@ -73,11 +72,7 @@ func TestConfigValidate(t *testing.T) { tt.cfg(&cfg) err := Validate(&cfg) - if tt.expectedErr != nil { - require.True(t, errors.Is(err, tt.expectedErr)) - } else { - require.NoError(t, err) - } + require.ErrorIs(t, err, tt.expectedErr) }) } } diff --git a/internal/httpfs/http_fs_test.go b/internal/httpfs/http_fs_test.go index 25003726..10a1ef7b 100644 --- a/internal/httpfs/http_fs_test.go +++ b/internal/httpfs/http_fs_test.go @@ -2,6 +2,7 @@ package httpfs import ( "io" + "io/fs" "net/http" "net/url" "os" @@ -22,7 +23,7 @@ func TestFSOpen(t *testing.T) { allowedPaths []string fileName string expectedContent string - expectedErrMsg string + expectedErr error }{ "file_allowed_in_file_path": { allowedPaths: []string{wd + "/testdata"}, @@ -35,19 +36,19 @@ func TestFSOpen(t *testing.T) { expectedContent: "subdir/file2.txt\n", }, "file_not_in_allowed_path": { - allowedPaths: []string{wd + "/testdata/subdir"}, - fileName: wd + "/testdata/file1.txt", - expectedErrMsg: os.ErrPermission.Error(), + allowedPaths: []string{wd + "/testdata/subdir"}, + fileName: wd + "/testdata/file1.txt", + expectedErr: fs.ErrPermission, }, "file_does_not_exist": { - allowedPaths: []string{wd + "/testdata"}, - fileName: wd + "/testdata/unknown.txt", - expectedErrMsg: "no such file or directory", + allowedPaths: []string{wd + "/testdata"}, + fileName: wd + "/testdata/unknown.txt", + expectedErr: fs.ErrNotExist, }, "relative_path_not_allowed": { - allowedPaths: []string{"testdata"}, - fileName: "testdata/file1.txt", - expectedErrMsg: os.ErrPermission.Error(), + allowedPaths: []string{"testdata"}, + fileName: "testdata/file1.txt", + expectedErr: fs.ErrPermission, }, "dot_dot_in_file_resolved": { allowedPaths: []string{wd + "/testdata"}, @@ -55,9 +56,9 @@ func TestFSOpen(t *testing.T) { expectedContent: "file1.txt\n", }, "dot_dot_in_file_resolved_not_allowed": { - allowedPaths: []string{wd + "/testdata/subdir"}, - fileName: wd + "/../httpfs/testdata/file1.txt", - expectedErrMsg: os.ErrPermission.Error(), + allowedPaths: []string{wd + "/testdata/subdir"}, + fileName: wd + "/../httpfs/testdata/file1.txt", + expectedErr: fs.ErrPermission, }, } @@ -67,9 +68,8 @@ func TestFSOpen(t *testing.T) { require.NoError(t, err) got, err := p.Open(test.fileName) - if test.expectedErrMsg != "" { - require.Error(t, err) - require.Contains(t, err.Error(), test.expectedErrMsg) + if test.expectedErr != nil { + require.ErrorIs(t, err, test.expectedErr) return } diff --git a/internal/httprange/http_ranged_reader_test.go b/internal/httprange/http_ranged_reader_test.go index 22f2b094..041232fc 100644 --- a/internal/httprange/http_ranged_reader_test.go +++ b/internal/httprange/http_ranged_reader_test.go @@ -97,7 +97,7 @@ func TestSectionReader(t *testing.T) { buf := make([]byte, tt.readSize) n, err := s.Read(buf) if tt.expectedErr != nil && !errors.Is(err, io.EOF) { - require.EqualError(t, err, tt.expectedErr.Error()) + require.ErrorIs(t, err, tt.expectedErr) return } @@ -179,7 +179,7 @@ func TestReadAt(t *testing.T) { n, err := reader.ReadAt(buf, int64(tt.sectionOffset)) if tt.expectedErr != nil { - require.EqualError(t, err, tt.expectedErr.Error()) + require.ErrorIs(t, err, tt.expectedErr) return } @@ -265,8 +265,7 @@ func TestReadContextCanceled(t *testing.T) { buf := make([]byte, resource.Size) n, err := s.Read(buf) - require.Error(t, err) - require.Contains(t, err.Error(), "context canceled") + require.ErrorIs(t, err, context.Canceled) require.Zero(t, n) }) @@ -274,8 +273,7 @@ func TestReadContextCanceled(t *testing.T) { rr.WithCachedReader(ctx, func() { buf := make([]byte, resource.Size) n, err := rr.ReadAt(buf, int64(0)) - require.Error(t, err) - require.Contains(t, err.Error(), "context canceled") + require.ErrorIs(t, err, context.Canceled) require.Zero(t, n) }) }) diff --git a/internal/httptransport/transport_test.go b/internal/httptransport/transport_test.go index feaf63b6..a51e7a02 100644 --- a/internal/httptransport/transport_test.go +++ b/internal/httptransport/transport_test.go @@ -2,7 +2,6 @@ package httptransport import ( "context" - "errors" "fmt" "net/http" "net/http/httptest" @@ -92,7 +91,7 @@ func TestRoundTripTTFBTimeout(t *testing.T) { res, err := mtr.RoundTrip(req) require.Nil(t, res) - require.True(t, errors.Is(err, context.Canceled), "context must have been canceled after ttfb timeout") + require.ErrorIs(t, err, context.Canceled, "context must have been canceled after ttfb timeout") } func newTestMetrics(t *testing.T) (*prometheus.HistogramVec, *prometheus.CounterVec) { diff --git a/internal/redirects/redirects_test.go b/internal/redirects/redirects_test.go index 5fdf8325..c0af60a7 100644 --- a/internal/redirects/redirects_test.go +++ b/internal/redirects/redirects_test.go @@ -30,7 +30,7 @@ func TestRedirectsRewrite(t *testing.T) { rule string expectedURL string expectedStatus int - expectedErr string + expectedErr error }{ { name: "No rules given", @@ -38,7 +38,7 @@ func TestRedirectsRewrite(t *testing.T) { rule: "", expectedURL: "", expectedStatus: 0, - expectedErr: ErrNoRedirect.Error(), + expectedErr: ErrNoRedirect, }, { name: "No matching rules", @@ -46,7 +46,7 @@ func TestRedirectsRewrite(t *testing.T) { rule: "/cake-portal.html /still-alive.html 301", expectedURL: "", expectedStatus: 0, - expectedErr: ErrNoRedirect.Error(), + expectedErr: ErrNoRedirect, }, { name: "Matching rule redirects", @@ -54,7 +54,6 @@ func TestRedirectsRewrite(t *testing.T) { rule: "/cake-portal.html /still-alive.html 301", expectedURL: "/still-alive.html", expectedStatus: http.StatusMovedPermanently, - expectedErr: "", }, { name: "Does not redirect to invalid rule", @@ -62,7 +61,7 @@ func TestRedirectsRewrite(t *testing.T) { rule: "/goto.html GitLab.com 301", expectedURL: "", expectedStatus: 0, - expectedErr: ErrNoRedirect.Error(), + expectedErr: ErrNoRedirect, }, { name: "Matches trailing slash rule to no trailing slash URL", @@ -70,7 +69,6 @@ func TestRedirectsRewrite(t *testing.T) { rule: "/cake-portal/ /still-alive/ 301", expectedURL: "/still-alive/", expectedStatus: http.StatusMovedPermanently, - expectedErr: "", }, { name: "Matches trailing slash rule to trailing slash URL", @@ -78,7 +76,6 @@ func TestRedirectsRewrite(t *testing.T) { rule: "/cake-portal/ /still-alive/ 301", expectedURL: "/still-alive/", expectedStatus: http.StatusMovedPermanently, - expectedErr: "", }, { name: "Matches no trailing slash rule to no trailing slash URL", @@ -86,7 +83,6 @@ func TestRedirectsRewrite(t *testing.T) { rule: "/cake-portal /still-alive 301", expectedURL: "/still-alive", expectedStatus: http.StatusMovedPermanently, - expectedErr: "", }, { name: "Matches no trailing slash rule to trailing slash URL", @@ -94,7 +90,6 @@ func TestRedirectsRewrite(t *testing.T) { rule: "/cake-portal /still-alive 301", expectedURL: "/still-alive", expectedStatus: http.StatusMovedPermanently, - expectedErr: "", }, { name: "matches_splat_rule", @@ -102,7 +97,6 @@ func TestRedirectsRewrite(t *testing.T) { rule: "/the-cake/* /is-a-lie 200", expectedURL: "/is-a-lie", expectedStatus: http.StatusOK, - expectedErr: "", }, { name: "replaces_splat_placeholdes", @@ -110,7 +104,6 @@ func TestRedirectsRewrite(t *testing.T) { rule: "/from/*/path /to/:splat/path 200", expectedURL: "/to/weighted/companion/cube/path", expectedStatus: http.StatusOK, - expectedErr: "", }, { name: "matches_placeholder_rule", @@ -118,7 +111,6 @@ func TestRedirectsRewrite(t *testing.T) { rule: "/the/:placeholder/is/delicious /the/:placeholder/is/a/lie 200", expectedURL: "/the/cake/is/a/lie", expectedStatus: http.StatusOK, - expectedErr: "", }, } @@ -144,12 +136,7 @@ func TestRedirectsRewrite(t *testing.T) { } require.Equal(t, tt.expectedStatus, status) - - if tt.expectedErr != "" { - require.EqualError(t, err, tt.expectedErr) - } else { - require.NoError(t, err) - } + require.ErrorIs(t, err, tt.expectedErr) }) } } @@ -163,31 +150,29 @@ func TestRedirectsParseRedirects(t *testing.T) { name string redirectsFile string expectedRules int - expectedErr string + expectedErr error }{ { name: "No `_redirects` file present", redirectsFile: "", expectedRules: 0, - expectedErr: errConfigNotFound.Error(), + expectedErr: errConfigNotFound, }, { name: "Everything working as expected", redirectsFile: `/goto.html /target.html 301`, expectedRules: 1, - expectedErr: "", }, { name: "Invalid _redirects syntax gives no rules", redirectsFile: `foobar::baz`, expectedRules: 0, - expectedErr: "", }, { name: "Config file too big", redirectsFile: strings.Repeat("a", 2*maxConfigSize), expectedRules: 0, - expectedErr: errFileTooLarge.Error(), + expectedErr: errFileTooLarge, }, // In future versions of `github.com/tj/go-redirects`, // this may not throw a parsing error and this test could be removed @@ -195,7 +180,7 @@ func TestRedirectsParseRedirects(t *testing.T) { name: "Parsing error is caught", redirectsFile: "/store id=:id /blog/:id 301", expectedRules: 0, - expectedErr: errFailedToParseConfig.Error(), + expectedErr: errFailedToParseConfig, }, } @@ -208,12 +193,7 @@ func TestRedirectsParseRedirects(t *testing.T) { redirects := ParseRedirects(ctx, root) - if tt.expectedErr != "" { - require.EqualError(t, redirects.error, tt.expectedErr) - } else { - require.NoError(t, redirects.error) - } - + require.ErrorIs(t, redirects.error, tt.expectedErr) require.Len(t, redirects.rules, tt.expectedRules) }) } @@ -230,14 +210,14 @@ func TestMaxRuleCount(t *testing.T) { redirects := ParseRedirects(context.Background(), root) - testFn := func(path, expectedToURL string, expectedStatus int, expectedErr string) func(t *testing.T) { + testFn := func(path, expectedToURL string, expectedStatus int, expectedErr error) func(t *testing.T) { return func(t *testing.T) { originalURL, err := url.Parse(path) require.NoError(t, err) toURL, status, err := redirects.Rewrite(originalURL) - if expectedErr != "" { - require.EqualError(t, err, expectedErr) + if expectedErr != nil { + require.ErrorIs(t, err, expectedErr) return } @@ -248,6 +228,6 @@ func TestMaxRuleCount(t *testing.T) { } } - t.Run("maxRuleCount matches", testFn("/1000.html", "/target1000", http.StatusMovedPermanently, "")) - t.Run("maxRuleCount+1 does not match", testFn("/1001.html", "", 0, ErrNoRedirect.Error())) + t.Run("maxRuleCount matches", testFn("/1000.html", "/target1000", http.StatusMovedPermanently, nil)) + t.Run("maxRuleCount+1 does not match", testFn("/1001.html", "", 0, ErrNoRedirect)) } diff --git a/internal/redirects/validations_test.go b/internal/redirects/validations_test.go index 9d891ece..bd108e6c 100644 --- a/internal/redirects/validations_test.go +++ b/internal/redirects/validations_test.go @@ -13,55 +13,46 @@ func TestRedirectsValidateUrl(t *testing.T) { tests := map[string]struct { url string - expectedErr string + expectedErr error }{ "valid_url": { - url: "/goto.html", - expectedErr: "", + url: "/goto.html", }, "no_domain_level_redirects": { url: "https://GitLab.com", - expectedErr: errNoDomainLevelRedirects.Error(), + expectedErr: errNoDomainLevelRedirects, }, "no_schemaless_url_domain_level_redirects": { url: "//GitLab.com/pages.html", - expectedErr: errNoDomainLevelRedirects.Error(), + expectedErr: errNoDomainLevelRedirects, }, "no_bare_domain_level_redirects": { url: "GitLab.com", - expectedErr: errNoStartingForwardSlashInURLPath.Error(), + expectedErr: errNoStartingForwardSlashInURLPath, }, "no_parent_traversing_relative_url": { url: "../target.html", - expectedErr: errNoStartingForwardSlashInURLPath.Error(), + expectedErr: errNoStartingForwardSlashInURLPath, }, "too_many_slashes": { url: strings.Repeat("/a", 26), - expectedErr: errTooManyPathSegments.Error(), + expectedErr: errTooManyPathSegments, }, "placeholders": { - url: "/news/:year/:month/:date/:slug", - expectedErr: "", + url: "/news/:year/:month/:date/:slug", }, "splats": { - url: "/blog/*", - expectedErr: "", + url: "/blog/*", }, "splat_placeholders": { - url: "/new/path/:splat", - expectedErr: "", + url: "/new/path/:splat", }, } for name, tt := range tests { t.Run(name, func(t *testing.T) { err := validateURL(tt.url) - if tt.expectedErr != "" { - require.EqualError(t, err, tt.expectedErr) - return - } - - require.NoError(t, err) + require.ErrorIs(t, err, tt.expectedErr) }) } } @@ -72,27 +63,22 @@ func TestRedirectsValidateUrl(t *testing.T) { func TestRedirectsValidateUrlNoPlaceholders(t *testing.T) { tests := map[string]struct { url string - expectedErr string + expectedErr error }{ "no_splats": { url: "/blog/*", - expectedErr: errNoSplats.Error(), + expectedErr: errNoSplats, }, "no_placeholders": { url: "/news/:year/:month/:date/:slug", - expectedErr: errNoPlaceholders.Error(), + expectedErr: errNoPlaceholders, }, } for name, tt := range tests { t.Run(name, func(t *testing.T) { err := validateURL(tt.url) - if tt.expectedErr != "" { - require.EqualError(t, err, tt.expectedErr) - return - } - - require.NoError(t, err) + require.ErrorIs(t, err, tt.expectedErr) }) } } @@ -102,31 +88,30 @@ func TestRedirectsValidateRule(t *testing.T) { tests := map[string]struct { rule string - expectedErr string + expectedErr error }{ "valid_rule": { - rule: "/goto.html /target.html 301", - expectedErr: "", + rule: "/goto.html /target.html 301", }, "invalid_from_url": { rule: "invalid.com /teapot.html 302", - expectedErr: errNoStartingForwardSlashInURLPath.Error(), + expectedErr: errNoStartingForwardSlashInURLPath, }, "invalid_to_url": { rule: "/goto.html invalid.com", - expectedErr: errNoStartingForwardSlashInURLPath.Error(), + expectedErr: errNoStartingForwardSlashInURLPath, }, "no_parameters": { rule: "/ /something 302 foo=bar", - expectedErr: errNoParams.Error(), + expectedErr: errNoParams, }, "invalid_status": { rule: "/goto.html /target.html 418", - expectedErr: errUnsupportedStatus.Error(), + expectedErr: errUnsupportedStatus, }, "force_not_supported": { rule: "/goto.html /target.html 302!", - expectedErr: errNoForce.Error(), + expectedErr: errNoForce, }, } @@ -136,12 +121,7 @@ func TestRedirectsValidateRule(t *testing.T) { require.NoError(t, err) err = validateRule(rules[0]) - if tt.expectedErr != "" { - require.EqualError(t, err, tt.expectedErr) - return - } - - require.NoError(t, err) + require.ErrorIs(t, err, tt.expectedErr) }) } } diff --git a/internal/source/gitlab/cache/cache_test.go b/internal/source/gitlab/cache/cache_test.go index 6eb48911..8b37ffc6 100644 --- a/internal/source/gitlab/cache/cache_test.go +++ b/internal/source/gitlab/cache/cache_test.go @@ -221,7 +221,7 @@ func TestResolve(t *testing.T) { lookup := cache.Resolve(context.Background(), "my.gitlab.com") require.Equal(t, 0, len(resolver.lookups)) - require.EqualError(t, lookup.Error, "retrieval context done: "+context.DeadlineExceeded.Error()) + require.ErrorIs(t, lookup.Error, context.DeadlineExceeded) }) }) @@ -235,7 +235,7 @@ func TestResolve(t *testing.T) { resolver.domain <- "err.gitlab.com" require.Equal(t, "my.gitlab.com", lookup.Name) - require.EqualError(t, lookup.Error, "context done: "+context.Canceled.Error()) + require.ErrorIs(t, lookup.Error, context.Canceled) }) }) }) diff --git a/internal/source/gitlab/client/client_test.go b/internal/source/gitlab/client/client_test.go index 131ddd9d..01e382d1 100644 --- a/internal/source/gitlab/client/client_test.go +++ b/internal/source/gitlab/client/client_test.go @@ -3,7 +3,6 @@ package client import ( "context" "encoding/base64" - "errors" "fmt" "net/http" "net/http/httptest" @@ -179,7 +178,7 @@ func TestMissingDomain(t *testing.T) { lookup := client.GetLookup(context.Background(), "group.gitlab.io") - require.True(t, errors.Is(lookup.Error, domain.ErrDomainDoesNotExist)) + require.ErrorIs(t, lookup.Error, domain.ErrDomainDoesNotExist) require.Nil(t, lookup.Domain) } @@ -273,11 +272,11 @@ func defaultClient(t *testing.T, url string) *Client { // prove fix for https://gitlab.com/gitlab-org/gitlab-pages/-/issues/587 func Test_endpoint(t *testing.T) { tests := map[string]struct { - basePath string - urlPath string - params url.Values - expectedURL string - expectedErrMsg string + basePath string + urlPath string + params url.Values + expectedURL string + expectedErr error }{ "all_slashes": { basePath: "/", @@ -317,9 +316,9 @@ func Test_endpoint(t *testing.T) { expectedURL: "https://gitlab.com/some/path", }, "url_path_is_not_a_url": { - basePath: "https://gitlab.com", - urlPath: "%", - expectedErrMsg: `parse "%": invalid URL escape "%"`, + basePath: "https://gitlab.com", + urlPath: "%", + expectedErr: url.EscapeError("%"), }, } @@ -329,9 +328,8 @@ func Test_endpoint(t *testing.T) { require.NoError(t, err) got, err := gc.endpoint(tt.urlPath, tt.params) - if tt.expectedErrMsg != "" { - require.Error(t, err) - require.Contains(t, err.Error(), tt.expectedErrMsg) + if tt.expectedErr != nil { + require.ErrorIs(t, err, tt.expectedErr) return } diff --git a/internal/source/gitlab/factory_test.go b/internal/source/gitlab/factory_test.go index 705fdc47..78d17ad7 100644 --- a/internal/source/gitlab/factory_test.go +++ b/internal/source/gitlab/factory_test.go @@ -54,7 +54,7 @@ func TestFabricateServing(t *testing.T) { Source: api.Source{Type: "file"}, } srv, err := g.fabricateServing(lookup) - require.EqualError(t, err, ErrDiskDisabled.Error()) + require.ErrorIs(t, err, ErrDiskDisabled) require.Nil(t, srv) }) } diff --git a/internal/vfs/local/root_test.go b/internal/vfs/local/root_test.go index 094a48c0..9b8cfe23 100644 --- a/internal/vfs/local/root_test.go +++ b/internal/vfs/local/root_test.go @@ -2,11 +2,11 @@ package local import ( "context" - "errors" "io" "io/fs" "os" "path/filepath" + "syscall" "testing" "github.com/stretchr/testify/assert" @@ -70,25 +70,25 @@ func TestReadlink(t *testing.T) { tests := map[string]struct { path string expectedTarget string - expectedErr string + expectedErr error expectedInvalidPath bool - expectedIsNotExist bool }{ "a valid link": { path: "testdata/link", expectedTarget: "file", }, "a file": { - path: "testdata/file", - expectedErr: "invalid argument", + path: "testdata/file", + // TODO: use fs.ErrInvalid once https://github.com/golang/go/issues/30322 is fixed + expectedErr: syscall.EINVAL, }, "a path outside of root directory": { path: "testdata/../../link", expectedInvalidPath: true, }, "a non-existing link": { - path: "non-existing", - expectedIsNotExist: true, + path: "non-existing", + expectedErr: fs.ErrNotExist, }, } @@ -96,19 +96,13 @@ func TestReadlink(t *testing.T) { t.Run(name, func(t *testing.T) { target, err := root.Readlink(ctx, test.path) - if test.expectedIsNotExist { - require.Equal(t, test.expectedIsNotExist, errors.Is(err, fs.ErrNotExist), "IsNotExist") - return - } - if test.expectedInvalidPath { require.IsType(t, &invalidPathError{}, err, "InvalidPath") return } - if test.expectedErr != "" { - require.Error(t, err) - require.Contains(t, err.Error(), test.expectedErr, "Readlink") + if test.expectedErr != nil { + require.ErrorIs(t, err, test.expectedErr, "Readlink") return } @@ -176,7 +170,7 @@ func TestLstat(t *testing.T) { modePerm os.FileMode modeType os.FileMode expectedInvalidPath bool - expectedIsNotExist bool + expectedErr error }{ "a directory": { path: "testdata", @@ -198,8 +192,8 @@ func TestLstat(t *testing.T) { expectedInvalidPath: true, }, "a non-existing link": { - path: "non-existing", - expectedIsNotExist: true, + path: "non-existing", + expectedErr: fs.ErrNotExist, }, } @@ -211,8 +205,8 @@ func TestLstat(t *testing.T) { fi, err := root.Lstat(ctx, test.path) - if test.expectedIsNotExist { - require.Equal(t, test.expectedIsNotExist, errors.Is(err, fs.ErrNotExist), "IsNotExist") + if test.expectedErr != nil { + require.ErrorIs(t, err, test.expectedErr) return } @@ -238,9 +232,8 @@ func TestOpen(t *testing.T) { tests := map[string]struct { path string expectedInvalidPath bool - expectedIsNotExist bool expectedContent string - expectedErr string + expectedErr error }{ "a file": { path: "testdata/file", @@ -248,19 +241,19 @@ func TestOpen(t *testing.T) { }, "a directory": { path: "testdata", - expectedErr: errNotFile.Error(), + expectedErr: errNotFile, }, "a link": { path: "testdata/link", - expectedErr: "too many levels of symbolic links", + expectedErr: syscall.ELOOP, }, "a path outside of root directory": { path: "testdata/../../link", expectedInvalidPath: true, }, "a non-existing file": { - path: "non-existing", - expectedIsNotExist: true, + path: "non-existing", + expectedErr: fs.ErrNotExist, }, } @@ -271,14 +264,8 @@ func TestOpen(t *testing.T) { defer file.Close() } - if test.expectedIsNotExist { - require.Equal(t, test.expectedIsNotExist, errors.Is(err, fs.ErrNotExist), "IsNotExist") - return - } - - if test.expectedErr != "" { - require.Error(t, err, "Open") - require.Contains(t, err.Error(), test.expectedErr, "Open") + if test.expectedErr != nil { + require.ErrorIs(t, err, test.expectedErr, "Open") return } diff --git a/internal/vfs/local/vfs_test.go b/internal/vfs/local/vfs_test.go index fd36dd4c..19840a22 100644 --- a/internal/vfs/local/vfs_test.go +++ b/internal/vfs/local/vfs_test.go @@ -2,7 +2,6 @@ package local import ( "context" - "errors" "io/fs" "os" "path/filepath" @@ -56,10 +55,9 @@ func TestVFSRoot(t *testing.T) { } tests := map[string]struct { - path string - expectedPath string - expectedErr error - expectedIsNotExist bool + path string + expectedPath string + expectedErr error }{ "a valid directory": { path: "dir", @@ -86,8 +84,8 @@ func TestVFSRoot(t *testing.T) { expectedErr: errNotDirectory, }, "a non-existing file": { - path: "not-existing", - expectedIsNotExist: true, + path: "not-existing", + expectedErr: fs.ErrNotExist, }, } @@ -95,13 +93,8 @@ func TestVFSRoot(t *testing.T) { t.Run(name, func(t *testing.T) { rootVFS, err := localVFS.Root(context.Background(), filepath.Join(tmpDir, test.path), "") - if test.expectedIsNotExist { - require.Equal(t, test.expectedIsNotExist, errors.Is(err, fs.ErrNotExist)) - return - } - if test.expectedErr != nil { - require.EqualError(t, err, test.expectedErr.Error()) + require.ErrorIs(t, err, test.expectedErr) return } diff --git a/internal/vfs/zip/archive_test.go b/internal/vfs/zip/archive_test.go index 37529e22..782c43b1 100644 --- a/internal/vfs/zip/archive_test.go +++ b/internal/vfs/zip/archive_test.go @@ -7,6 +7,7 @@ import ( "crypto/rand" "fmt" "io" + "io/fs" "net/http" "net/http/httptest" "os" @@ -66,7 +67,7 @@ func testOpen(t *testing.T, zip *zipArchive) { }, "file_does_not_exist": { file: "unknown.html", - expectedErr: os.ErrNotExist, + expectedErr: fs.ErrNotExist, }, } @@ -74,7 +75,7 @@ func testOpen(t *testing.T, zip *zipArchive) { t.Run(name, func(t *testing.T) { f, err := zip.Open(context.Background(), tt.file) if tt.expectedErr != nil { - require.EqualError(t, err, tt.expectedErr.Error()) + require.ErrorIs(t, err, tt.expectedErr) return } @@ -248,7 +249,7 @@ func testLstat(t *testing.T, zip *zipArchive) { }, "file_does_not_exist": { file: "unknown.html", - expectedErr: os.ErrNotExist, + expectedErr: fs.ErrNotExist, }, } @@ -256,7 +257,7 @@ func testLstat(t *testing.T, zip *zipArchive) { t.Run(name, func(t *testing.T) { fi, err := zip.Lstat(context.Background(), tt.file) if tt.expectedErr != nil { - require.EqualError(t, err, tt.expectedErr.Error()) + require.ErrorIs(t, err, tt.expectedErr) return } @@ -309,7 +310,7 @@ func testReadLink(t *testing.T, zip *zipArchive) { }, "file_does_not_exist": { file: "unknown.html", - expectedErr: os.ErrNotExist, + expectedErr: fs.ErrNotExist, }, } @@ -317,7 +318,7 @@ func testReadLink(t *testing.T, zip *zipArchive) { t.Run(name, func(t *testing.T) { link, err := zip.Readlink(context.Background(), tt.file) if tt.expectedErr != nil { - require.EqualError(t, err, tt.expectedErr.Error()) + require.ErrorIs(t, err, tt.expectedErr) return } @@ -357,7 +358,7 @@ func TestArchiveCanBeReadAfterOpenCtxCanceled(t *testing.T) { cancel() err := zip.openArchive(ctx, testServerURL+"/public.zip") - require.EqualError(t, err, context.Canceled.Error()) + require.ErrorIs(t, err, context.Canceled) <-zip.done @@ -374,15 +375,14 @@ func TestReadArchiveFails(t *testing.T) { testServerURL, cleanup := newZipFileServerURL(t, "group/zip.gitlab.io/public.zip", nil) defer cleanup() - fs := New(&zipCfg).(*zipVFS) - zip := newArchive(fs, time.Second) + zfs := New(&zipCfg).(*zipVFS) + zip := newArchive(zfs, time.Second) err := zip.openArchive(context.Background(), testServerURL+"/unkown.html") - require.Error(t, err) - require.Contains(t, err.Error(), httprange.ErrNotFound.Error()) + require.ErrorIs(t, err, httprange.ErrNotFound) _, err = zip.Open(context.Background(), "index.html") - require.EqualError(t, err, os.ErrNotExist.Error()) + require.ErrorIs(t, err, fs.ErrNotExist) } func createArchive(t *testing.T, dir string) (map[string][]byte, int64) { diff --git a/internal/vfs/zip/vfs_test.go b/internal/vfs/zip/vfs_test.go index 12e82c1c..af1d263b 100644 --- a/internal/vfs/zip/vfs_test.go +++ b/internal/vfs/zip/vfs_test.go @@ -5,6 +5,7 @@ import ( "errors" "io" "io/fs" + "net/url" "testing" "time" @@ -17,27 +18,27 @@ import ( ) func TestVFSRoot(t *testing.T) { - url, cleanup := newZipFileServerURL(t, "group/zip.gitlab.io/public.zip", nil) + u, cleanup := newZipFileServerURL(t, "group/zip.gitlab.io/public.zip", nil) defer cleanup() tests := map[string]struct { - path string - sha256 string - expectedErrMsg string + path string + sha256 string + expectedErr error }{ "zip_file_exists": { path: "/public.zip", sha256: "d6b318b399cfe9a1c8483e49847ee49a2676d8cfd6df57ec64d971ad03640a75", }, "zip_file_does_not_exist": { - path: "/unknown", - sha256: "filedoesnotexist", - expectedErrMsg: fs.ErrNotExist.Error(), + path: "/unknown", + sha256: "filedoesnotexist", + expectedErr: fs.ErrNotExist, }, "invalid_url": { - path: "/%", - sha256: "invalidurl", - expectedErrMsg: "invalid URL", + path: "/%", + sha256: "invalidurl", + expectedErr: url.EscapeError("%"), }, } @@ -45,10 +46,9 @@ func TestVFSRoot(t *testing.T) { for name, tt := range tests { t.Run(name, func(t *testing.T) { - root, err := vfs.Root(context.Background(), url+tt.path, tt.sha256) - if tt.expectedErrMsg != "" { - require.Error(t, err) - require.Contains(t, err.Error(), tt.expectedErrMsg) + root, err := vfs.Root(context.Background(), u+tt.path, tt.sha256) + if tt.expectedErr != nil { + require.ErrorIs(t, err, tt.expectedErr) return } |