-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
add sweepers auto-resolver code #12950
base: main
Are you sure you want to change the base?
add sweepers auto-resolver code #12950
Conversation
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR hasn't generated any diffs, but I'll let you know if a future commit does. |
Tests analyticsTotal tests: 1685 Click here to see the affected service packages
Action takenFound 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
🔴 Tests failed during RECORDING mode: 🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR. |
log.Fatal("Please provide a directory path using -dir flag") | ||
} | ||
|
||
allTests, errs := reader.ReadAllTests(*dirPath) |
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.
Ideally we'd want this to be integrated in the CI pipeline, which is already reading tests in order to do missing test detection. It would be great to only read them once, then do all the relevant processing of that data (for missing tests, sweeper regions, and sweepable prefix detection), and post any suggested changes as inline comments (or comments on the file in cases where inline doesn't work for some reason).
SourceFile string `yaml:"source_file,omitempty"` | ||
} | ||
|
||
func main() { |
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.
We should run this using cobra to match our other tooling and because it's more readable and abstracts away some of the weird stuff about golang CLI implementation. It should return the basic data about what issues were detected, which would get used by the CI to generate inline comments (ideally using some kind of generic implementation that could handle issue creation for missing test detector and sweepable prefixes as well - maybe also for other arbitrary feedback we might want to create in the future. For example, we could return an object that contains a message plus the line number and file to attach it to. Any multi-line message should most likely be built using a go template.)
This is pretty high level feedback so I'm not getting into details because I don't know how much will change - let me know if you have any questions! |
Adding missing sweeper regions and introduce auto resolver code. Note that I haven't added unit tests since this code is still evolving. Need to work this for the {{name}} and {{prefixes}}.
Release Note Template for Downstream PRs (will be copied)
See Write release notes for guidance.