-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
update linker tests to have descriptive names #27
Conversation
Update linker table driven tests to have descriptive names and run with t.Run (for easier targeting/debugging of a single test). Enable the 'paralleltest' linter which will ensure that all tests can run in parallel and we properly reassign variables inside a loop so they run properly in parallel.
@@ -58,7 +58,6 @@ linters: | |||
- nilerr | |||
- nilnil | |||
- nonamedreturns | |||
- paralleltest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enabled every test to run in parallel.
}, | ||
{ | ||
map[string]string{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test was duplicated (same input) as another one above so removed it.
if tc.errMsg == "" { | ||
if err != nil { | ||
t.Errorf("case %d: expecting no error; instead got error %q", i, err) | ||
assert.Truef(t, strings.HasPrefix(name, expectedPrefix), "expected test name %q to have %q prefix", name, expectedPrefix) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This felt nice to enforce some consistency in naming.
type msg struct { | ||
pos ast.SourcePos | ||
text string | ||
} | ||
var msgs []msg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved this into the tests - otherwise it led to a race condition.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great!
Co-authored-by: Joshua Humphries <jhumphries@buf.build>
Update linker table driven tests to have descriptive names and run with t.Run (for easier targeting/debugging of a single test). Enable the 'paralleltest' linter which will ensure that all tests can run in parallel and we properly reassign variables inside a loop so they run properly in parallel.