-
Notifications
You must be signed in to change notification settings - Fork 4.4k
Streamline reduce_tests.sh a bit #82963
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
Conversation
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.
Auto-requesting reviews from non-collaborators: @jbytheway
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 is a good add! I've not had much of a problem with misc failures polluting the tool, but it has happened once or twice.
It's also good to see I'm not the only one who uses this :)
FWIW, the reason I used But I'm also glad to see this script is still seeing some use. |
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.
I think this is portable.
Thanks! I've just been trying to keep focused on hordes so neglecting this a bit, but I'll go ahead and merge this in. |
Swaps from multidelta that makes things more difficult without giving any new features to singledelta Adds a stub showing how to test for a target output string instead of test success
2348bb1
to
12ba3c0
Compare
Summary
None
Purpose of change
When I tried to use reduce_tests.sh on #81077 I found it to be quite fragile and require a lot of babysitting for false positives (a positive in this case is the test fails in an expected way).
Also I found that the multidelta wrapper script was re-running the test suite redundantly under the assumption that it was doing test case reduction across multiple files and using a tool to semantically group c/c++ expressions, neither of which we were doing.
Describe the solution
Adds a stub showing how to test for a target output string instead of test success. This is the recommended way to use delta to avoid false negatives, I think it can handle false negatives from "this makes the build fail" or "this makes the whole thing crash", but false positives seem to make it wander off into un-fruitful directions.
Swaps from multidelta that makes things slower without giving any new features to singledelta.
Describe alternatives you've considered
Ideally this would be goverened by an argument or something instead of requiring manual editing, but I really do not have the brain juice for that right now.
Since this isn't running through the "flatten" process any more, I don't think adding and removing curly brackets around the test cases is necessary anymore, but it's also not breaking anything so whatever.
Possibly the "match against a string" workflow should be the default, but that requires parameter handling so bleh.
Testing
Ran it a bunch of times against #81077 and it was pretty reliable once I had it searching for the error string.