Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
test(smokehouse): use static requires for test definitions #9501
test(smokehouse): use static requires for test definitions #9501
Changes from 4 commits
757e417
5924911
8acbcb1
a1bc19e
3756200
3236265
10cca97
f0f918e
20dd239
fbeae33
a83133f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
static require paths for the smoke tests I'm all for, but the new
{path, value}
structure of config seems unnecessarily confusing. would it be possible to move to the model in #8962 of colocating the config/expectations and staticly require'ing in that file? IIUC that should alleviate the need for path here since we move to the transient config files :)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.
Let's drop the path/value thing in favor of writing temporary files for the config when passing to the smokehouse runner. That interface was the only reason path is needed.
Colocating the expectation/config seems like a bigger change to me and not one that's needed for static resolving to work.