-
Notifications
You must be signed in to change notification settings - Fork 2k
feat: testing isolation #3872
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
feat: testing isolation #3872
Conversation
|
|
||
| func Affected(config Config, diffs []string) ([]string, error) { | ||
| changed := Changed(config, diffs) | ||
| if slices.Contains(changed, ".") { |
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.
Wow is this new fancy slices packages nice. Back in my day ... ;D.
[No action required]
telpirion
left a comment
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.
Looking really good! I will probably take another look at the parallelized code in RunAll() (but it looks solid prima facie).
Global: make sure that exported types, functions, and methods all have Godoc comments.
…ples into testing-isolation
telpirion
left a comment
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.
Very nicely done. One minor nit, but otherwise--let's ship it!
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.
nit (very minor): Go style is to keep tests and the code under test together.
| ) | ||
|
|
||
| func TestLoadConfig(t *testing.T) { | ||
| tests := []struct { |
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.
Great use of table testing!
|
Moved tests into the source directory, updated README as well. |
|
@gemini review |
1 similar comment
|
@gemini review |
Description
This is an effort to speed up checks for pull requests, while at the same time simplifying the existing testing infrastructure in Node.js by using a dynamic strategy matrix.
Note: We're currently not using the
run-allcommand, that's infrastructure that will eventually be used to optimize the nightly jobs.These new tests are marked as
experimental, so they shouldn't block any pull requests from merging. Once we have all the tests working on the new infrastructure and we feel comfortable with it, we can go ahead and disable the old tests.Checklist
npm test(see Testing)npm run lint(see Style)GoogleCloudPlatform/nodejs-docs-samples. Not a fork.