Cleanup lint issues with uses of testify in Go code
Ran testifylint (https://github.com/Antonboom/testifylint) over our
tests to see what issues it would detect, since testify has some
inconsistent APIs, e.g. .Equal(expect, real), but .Len(obj, count), so
it is easy to get args reversed or miss a better function to use.
It found mostly minor things, like being able use .Len() instead of
.Equal(..., len(foo)) or using .NoError() instead of .Nil() on errors,
and a bunch of .Equal() calls that should be reversed.
It did get a number of things wrong that I had to revert, and missed a
number potential cleanups (like specific cases it could be using
.NoError() or .Len() even though it identified other cases in the same
file), so I don't think I would recommend us adding it to our
presubmit or CI.
It is a useful tool for checking your code style, but I don't think it
is good enough at this point to be blocking CLs or be run in an
automated fashion.
Change-Id: Ib44412442af82cf3ab3bf04de7c4d68fabb1f2ca
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/257896
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Auto-Submit: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: Brian Sheedy <bsheedy@google.com>
Commit-Queue: Brian Sheedy <bsheedy@google.com>
diff --git a/tools/src/auth/auth_test.go b/tools/src/auth/auth_test.go
index 69b40a5..702195a 100644
--- a/tools/src/auth/auth_test.go
+++ b/tools/src/auth/auth_test.go
@@ -43,7 +43,7 @@
authOptions := DefaultAuthOptions(wrapper)
- require.Equal(t, authOptions.SecretsDir, "/home/.config/dawn-cts")
+ require.Equal(t, "/home/.config/dawn-cts", authOptions.SecretsDir)
require.GreaterOrEqual(t, len(authOptions.Scopes), 2)
require.Equal(
t,
@@ -62,7 +62,7 @@
authOptions := DefaultAuthOptions(wrapper, "scope1", "scope2")
- require.Equal(t, authOptions.SecretsDir, "/home/.config/dawn-cts")
+ require.Equal(t, "/home/.config/dawn-cts", authOptions.SecretsDir)
require.GreaterOrEqual(t, len(authOptions.Scopes), 4)
require.Equal(
t,
diff --git a/tools/src/cmd/cts/common/paths_test.go b/tools/src/cmd/cts/common/paths_test.go
index 660cd86..a66e12b 100644
--- a/tools/src/cmd/cts/common/paths_test.go
+++ b/tools/src/cmd/cts/common/paths_test.go
@@ -67,7 +67,7 @@
t.Run(testCase.name, func(t *testing.T) {
wrapper, err := fileutils.CreateMemMapOSWrapperWithFakeDawnRoot()
require.NoErrorf(t, err, "Error creating fake Dawn root: %v", err)
- require.Equal(t, testCase.f(wrapper), "")
+ require.Equal(t, "", testCase.f(wrapper))
})
}
}
@@ -110,7 +110,7 @@
wrapper, err := CreateMemMapOSWrapperWithFakeDefaultPaths()
require.NoErrorf(t, err, "Error creating fake Dawn directories: %v", err)
expectationsPath := testCase.f(wrapper)
- require.NotEqual(t, expectationsPath, "")
+ require.NotEqual(t, "", expectationsPath)
require.True(t, strings.HasPrefix(expectationsPath, fileutils.DawnRoot(wrapper)))
require.True(t, strings.HasSuffix(expectationsPath, testCase.expectedSuffix))
})
@@ -120,15 +120,15 @@
func TestDefaultExpectationsPaths_PathDoesNotExist(t *testing.T) {
wrapper, err := fileutils.CreateMemMapOSWrapperWithFakeDawnRoot()
require.NoErrorf(t, err, "Error creating fake Dawn root: %v", err)
- require.Equal(t, DefaultExpectationsPaths(wrapper), []string{"", ""})
+ require.Equal(t, []string{"", ""}, DefaultExpectationsPaths(wrapper))
}
func TestDefaultExpectationsPaths_PathExists(t *testing.T) {
wrapper, err := CreateMemMapOSWrapperWithFakeDefaultPaths()
require.NoErrorf(t, err, "Error creating fake Dawn directories: %v", err)
expectationsPaths := DefaultExpectationsPaths(wrapper)
- require.NotEqual(t, expectationsPaths, []string{"", ""})
- require.Equal(t, len(expectationsPaths), 2)
+ require.NotEqual(t, []string{"", ""}, expectationsPaths)
+ require.Len(t, expectationsPaths, 2)
require.True(t, strings.HasPrefix(expectationsPaths[0], fileutils.DawnRoot(wrapper)))
require.True(t, strings.HasPrefix(expectationsPaths[1], fileutils.DawnRoot(wrapper)))
require.True(t, strings.HasSuffix(expectationsPaths[0], RelativeExpectationsPath))
diff --git a/tools/src/cmd/cts/common/results_test.go b/tools/src/cmd/cts/common/results_test.go
index 97a8cd3..1136077 100644
--- a/tools/src/cmd/cts/common/results_test.go
+++ b/tools/src/cmd/cts/common/results_test.go
@@ -377,8 +377,8 @@
expectedResults["execution_mode"] = expectedResultsList
results, err := GetResults(ctx, cfg, client, builds)
- require.Nil(t, err)
- require.Equal(t, results, expectedResults)
+ require.NoError(t, err)
+ require.Equal(t, expectedResults, results)
}
// Tests that errors from GetRawResults are properly surfaced.
@@ -487,8 +487,8 @@
expectedResults["execution_mode"] = expectedResultsList
results, err := GetUnsuppressedFailingResults(ctx, cfg, client, builds)
- require.Nil(t, err)
- require.Equal(t, results, expectedResults)
+ require.NoError(t, err)
+ require.Equal(t, expectedResults, results)
}
// Tests that errors from GetRawResults are properly surfaced.
@@ -591,8 +591,8 @@
expectedResults["execution_mode"] = expectedResultsList
results, err := GetRawResults(ctx, cfg, client, builds)
- require.Nil(t, err)
- require.Equal(t, results, expectedResults)
+ require.NoError(t, err)
+ require.Equal(t, expectedResults, results)
}
// Tests that a mismatched prefix results in an error.
@@ -775,8 +775,8 @@
expectedResults["execution_mode"] = expectedResultsList
results, err := GetRawUnsuppressedFailingResults(ctx, cfg, client, builds)
- require.Nil(t, err)
- require.Equal(t, results, expectedResults)
+ require.NoError(t, err)
+ require.Equal(t, expectedResults, results)
}
// Tests that a mismatched prefix results in an error.
@@ -868,7 +868,7 @@
}
CleanResults(cfg, &results)
- require.Equal(t, results, expectedResults)
+ require.Equal(t, expectedResults, results)
}
// Tests that duplicate results with the same status always use that status.
@@ -904,7 +904,7 @@
}
CleanResults(cfg, &results)
- require.Equal(t, results, expectedResults)
+ require.Equal(t, expectedResults, results)
}
}
@@ -939,7 +939,7 @@
}
CleanResults(cfg, &results)
- require.Equal(t, results, expectedResults)
+ require.Equal(t, expectedResults, results)
}
}
@@ -1005,7 +1005,7 @@
}
CleanResults(cfg, &results)
- require.Equal(t, results, expectedResults)
+ require.Equal(t, expectedResults, results)
}
}
}
@@ -1443,5 +1443,5 @@
resultsByExecutionMode, err := getRecentUniqueSuppressedResults(ctx, cfg, client)
require.NoErrorf(t, err, "Got error getting results: %v", err)
- require.Equal(t, resultsByExecutionMode, expectedResults)
+ require.Equal(t, expectedResults, resultsByExecutionMode)
}
diff --git a/tools/src/cmd/cts/expectationcoverage/expectationcoverage_test.go b/tools/src/cmd/cts/expectationcoverage/expectationcoverage_test.go
index 3a1af35..96c9cdb 100644
--- a/tools/src/cmd/cts/expectationcoverage/expectationcoverage_test.go
+++ b/tools/src/cmd/cts/expectationcoverage/expectationcoverage_test.go
@@ -208,7 +208,7 @@
/*verbose=*/ false,
wrapper)
- require.Equal(t, content, expectations.Content{})
+ require.Equal(t, expectations.Content{}, content)
require.ErrorContains(t, err, "open /expectations.txt: file does not exist")
}
@@ -230,7 +230,7 @@
/*verbose=*/ false,
wrapper)
- require.Equal(t, content, expectations.Content{})
+ require.Equal(t, expectations.Content{}, content)
require.ErrorContains(t, err, "/expectations.txt:6:31 error: expected status")
}
diff --git a/tools/src/cmd/gen/build/build_test.go b/tools/src/cmd/gen/build/build_test.go
index 7d8ec6b..70c550e 100644
--- a/tools/src/cmd/gen/build/build_test.go
+++ b/tools/src/cmd/gen/build/build_test.go
@@ -331,7 +331,7 @@
// We can't compare the entire structs to each other since
// includePatternMatch will be an anonymous function.
require.Equal(t, testCase.want.externalsJsonPath, p.externalsJsonPath)
- require.Equal(t, len(testCase.want.externals), len(p.externals))
+ require.Len(t, p.externals, len(testCase.want.externals))
for k := range p.externals {
require.Contains(t, testCase.want.externals, k)
require.Equal(t, testCase.want.externals[k].Name, p.externals[k].Name)
diff --git a/tools/src/cmd/run-cts/common/flags_test.go b/tools/src/cmd/run-cts/common/flags_test.go
index 533c934..87d9d3a 100644
--- a/tools/src/cmd/run-cts/common/flags_test.go
+++ b/tools/src/cmd/run-cts/common/flags_test.go
@@ -39,13 +39,13 @@
func TestDefaultCtsPath_NoDawnRoot(t *testing.T) {
wrapper := oswrapper.CreateMemMapOSWrapper()
- require.Equal(t, defaultCtsPath(wrapper), "")
+ require.Equal(t, "", defaultCtsPath(wrapper))
}
func TestDefaultCtsPath_NoCts(t *testing.T) {
wrapper, err := fileutils.CreateMemMapOSWrapperWithFakeDawnRoot()
require.NoErrorf(t, err, "Error creating fake Dawn root: %v", err)
- require.Equal(t, defaultCtsPath(wrapper), "")
+ require.Equal(t, "", defaultCtsPath(wrapper))
}
func TestDefaultCtsPath_CtsExists(t *testing.T) {
@@ -53,5 +53,5 @@
require.NoErrorf(t, err, "Error creating fake Dawn root: %v", err)
expectedPath := filepath.Join(fileutils.DawnRoot(wrapper), "third_party", "webgpu-cts")
wrapper.MkdirAll(expectedPath, 0o666)
- require.Equal(t, defaultCtsPath(wrapper), expectedPath)
+ require.Equal(t, expectedPath, defaultCtsPath(wrapper))
}
diff --git a/tools/src/cts/expectations/expectations_test.go b/tools/src/cts/expectations/expectations_test.go
index ce22b9a..bd9180e 100644
--- a/tools/src/cts/expectations/expectations_test.go
+++ b/tools/src/cts/expectations/expectations_test.go
@@ -208,7 +208,7 @@
Status: []string{"Failure", "Slow"},
Comment: "# comment",
}
- require.Equal(t, e.AsExpectationFileString(), "crbug.com/1234 [ linux nvidia ] query [ Failure Slow ] # comment")
+ require.Equal(t, "crbug.com/1234 [ linux nvidia ] query [ Failure Slow ] # comment", e.AsExpectationFileString())
// No bug.
e = Expectation{
@@ -217,7 +217,7 @@
Status: []string{"Failure", "Slow"},
Comment: "# comment",
}
- require.Equal(t, e.AsExpectationFileString(), "[ linux nvidia ] query [ Failure Slow ] # comment")
+ require.Equal(t, "[ linux nvidia ] query [ Failure Slow ] # comment", e.AsExpectationFileString())
// No tags.
e = Expectation{
@@ -227,7 +227,7 @@
Status: []string{"Failure", "Slow"},
Comment: "# comment",
}
- require.Equal(t, e.AsExpectationFileString(), "crbug.com/1234 query [ Failure Slow ] # comment")
+ require.Equal(t, "crbug.com/1234 query [ Failure Slow ] # comment", e.AsExpectationFileString())
// No comment.
e = Expectation{
@@ -236,14 +236,14 @@
Query: "query",
Status: []string{"Failure", "Slow"},
}
- require.Equal(t, e.AsExpectationFileString(), "crbug.com/1234 [ linux nvidia ] query [ Failure Slow ]")
+ require.Equal(t, "crbug.com/1234 [ linux nvidia ] query [ Failure Slow ]", e.AsExpectationFileString())
// Minimal expectation.
e = Expectation{
Query: "query",
Status: []string{"Failure", "Slow"},
}
- require.Equal(t, e.AsExpectationFileString(), "query [ Failure Slow ]")
+ require.Equal(t, "query [ Failure Slow ]", e.AsExpectationFileString())
}
func TestSort(t *testing.T) {
@@ -327,7 +327,7 @@
secondLinuxTwo,
}
- require.Equal(t, expectationsList, expectedList)
+ require.Equal(t, expectedList, expectationsList)
}
func TestSortPrioritizeQuery(t *testing.T) {
@@ -411,7 +411,7 @@
secondLinuxTwo,
}
- require.Equal(t, expectationsList, expectedList)
+ require.Equal(t, expectedList, expectationsList)
}
func TestIsGlobExpectation(t *testing.T) {
@@ -469,9 +469,9 @@
if testCase.isGlobExpected {
expectedType = GLOB
}
- require.Equal(t, testCase.e.expectationType, UNDETERMINED)
- require.Equal(t, testCase.e.IsGlobExpectation(), testCase.isGlobExpected)
- require.Equal(t, testCase.e.expectationType, expectedType)
+ require.Equal(t, UNDETERMINED, testCase.e.expectationType)
+ require.Equal(t, testCase.isGlobExpected, testCase.e.IsGlobExpectation())
+ require.Equal(t, expectedType, testCase.e.expectationType)
})
}
}
diff --git a/tools/src/glob/glob_test.go b/tools/src/glob/glob_test.go
index 910b7b8..7ca20a4 100644
--- a/tools/src/glob/glob_test.go
+++ b/tools/src/glob/glob_test.go
@@ -306,7 +306,7 @@
// We can't check equality since the loaded Config contains function
// pointers to anonymous functions. So, just check that the number of
// rules matches.
- require.Equal(t, testCase.wantNumRules, len(cfg.Paths))
+ require.Len(t, cfg.Paths, testCase.wantNumRules)
if testCase.wantErr {
require.ErrorContains(t, err, testCase.wantErrMsg)
} else {
diff --git a/tools/src/oswrapper/fstestoswrapper_test.go b/tools/src/oswrapper/fstestoswrapper_test.go
index 72b6831..86169fa 100644
--- a/tools/src/oswrapper/fstestoswrapper_test.go
+++ b/tools/src/oswrapper/fstestoswrapper_test.go
@@ -181,7 +181,7 @@
require.NoError(t, err)
testMap := testFS.FS
- require.Equal(t, len(realMap), len(testMap), "Filesystems have a different number of entries")
+ require.Len(t, testMap, len(realMap), "Filesystems have a different number of entries")
for key, realFile := range realMap {
testFile, ok := testMap[key]
@@ -288,7 +288,7 @@
t.Run("Get non-existent variable", func(t *testing.T) {
val := wrapper.Getenv("NON_EXISTENT_VAR")
- require.Equal(t, "", val)
+ require.Empty(t, val)
})
}
diff --git a/tools/src/oswrapper/memmaposwrapper_test.go b/tools/src/oswrapper/memmaposwrapper_test.go
index 8b8e4e8..8b93d1d 100644
--- a/tools/src/oswrapper/memmaposwrapper_test.go
+++ b/tools/src/oswrapper/memmaposwrapper_test.go
@@ -63,7 +63,7 @@
func TestMemMapOSWrapper_Getenv(t *testing.T) {
wrapper := CreateMemMapOSWrapper()
- require.Equal(t, "", wrapper.Getenv("HOME"))
+ require.Empty(t, wrapper.Getenv("HOME"))
wrapper.MemMapEnvironProvider.Environment = map[string]string{
"HOME": "/tmp",
@@ -158,7 +158,7 @@
walkfunc := func(root string, info fs.FileInfo, err error) error {
require.Equal(t, "/nonexistent", root)
- require.Equal(t, nil, info)
+ require.Nil(t, info)
require.ErrorContains(t, err, "open /nonexistent: file does not exist")
return err
}
@@ -203,7 +203,7 @@
err = wrapper.Walk("/parent", walkfunc)
require.NoErrorf(t, err, "Got error walking: %v", err)
- require.Equal(t, 2, len(inputs))
+ require.Len(t, inputs, 2)
require.Equal(t, "/parent", inputs[0].root)
require.True(t, inputs[0].info.IsDir())
require.Equal(t, "/parent/foo.txt", inputs[1].root)
@@ -295,7 +295,7 @@
err = wrapper.Walk("/parent", walkfunc)
require.NoErrorf(t, err, "Got error walking: %v", err)
- require.Equal(t, len(inputs), 3)
+ require.Len(t, inputs, 3)
require.Equal(t, "/parent", inputs[0].root)
require.True(t, inputs[0].info.IsDir())
diff --git a/tools/src/reducedglob/reducedglob_test.go b/tools/src/reducedglob/reducedglob_test.go
index 8553354..02c3c4d 100644
--- a/tools/src/reducedglob/reducedglob_test.go
+++ b/tools/src/reducedglob/reducedglob_test.go
@@ -134,7 +134,7 @@
for _, testCase := range tests {
t.Run(testCase.name, func(t *testing.T) {
g := NewReducedGlob(testCase.pattern)
- require.Equal(t, g.Matchcase(testCase.matchedString), testCase.expectedMatch)
+ require.Equal(t, testCase.expectedMatch, g.Matchcase(testCase.matchedString))
})
}
}
@@ -161,7 +161,7 @@
}
for _, testCase := range tests {
t.Run(testCase.name, func(t *testing.T) {
- require.Equal(t, findAllIndices(testCase.s, testCase.substr), testCase.expectedIndices)
+ require.Equal(t, testCase.expectedIndices, findAllIndices(testCase.s, testCase.substr))
})
}
}
