-
Notifications
You must be signed in to change notification settings - Fork 77
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
misc: add ShrinkException for easy test case shrinking #3610
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3610 +/- ##
==========================================
- Coverage 91.30% 91.29% -0.01%
==========================================
Files 468 468
Lines 58636 58646 +10
Branches 5656 5657 +1
==========================================
+ Hits 53535 53540 +5
- Misses 3650 3655 +5
Partials 1451 1451 ☔ View full report in Codecov by Sentry. |
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.
It's not really clear to me what this change does without an example. Are the printed strings magic keywords that shrinkray understands? Does it just use the program code output?
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.
Is this eager optimization (or debugging)?
It seems interesting, but I have the same questions as @alexarice.
Does it work like llvm-reduce
or bugpoint
but on the Python exception stack level?
Did it help you narrow down the above? Somehow, I've never had an issue with Python's exception unwinding.
Maybe I'm missing an example?
I'll add more documentation, here's an example, it shrunk the test case in the PR it sat on top of to the file in the PR: |
Thanks, I'll play with that and come back. |
What are you planning to use this for? To me, this is a tool that is primarily useful for bug reporting after finding a bug through some automated way (i.e., some form of fuzzing). I can see that supporting something to reduce testcases directly within xdsl might be useful for some people, but I am unsure whether this quite specific use case actually justifies putting this into xdsl-opt. Can't people just use shrinkray on their own and then report the bug triggering test case back to us? Does this really have to be deeply integrated into xdsl to be useable? |
So it's more of a convenience for when someone wants to use it. The shrinkray interface is you feed it a sccript that exits with 0 if the condition was met, and non-0 if it was not met. This is not necessarily easy to orchestrate for reducing a test-case, and I had to do a bit of leg work to make it work locally. With this change in, the effort required to create a script that shrinks correctly is minimised. |
I see. Does this make shrinkray a dependency? Or do only users that want to use this feature need it installed? I do not have a strong opinion on this. To me, it feels like a bit of an overfit for a specific use case, but if you believe this to be an important/nice use case, I am fine with it. |
I just think it's magic, and I'd love to be able to perform magic on other people's computers when they ask me how to reduce a test case :) |
I think this might come in handy in the future.
I opened a feature request to make it more ergonomic in the future: DRMacIver/shrinkray#11