-
Notifications
You must be signed in to change notification settings - Fork 18
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
Fix behaviour of fence and horizons in dry runs #196
Conversation
Check-perf-impact results: (9e900a306dc9f17e4a27439205a7680c) ❓ No new benchmark data submitted. ❓ |
(Note: this is currently failing due to our CI rework; will only merge after all of that is resolved) |
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.
LGTM! Also, libenvpp is 🔥.
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.
Good change!
Also add a unit test and get rid of some utilities that are no longer necessary.
Also added a unit test.
Note that I very minorly changed the existing dry run test: it was the only test to still use
test_utils::set_test_env
. Changing this to use the equivalentlibenvpp
functionality allowed me to get rid of the entireset_test_env
test utility (which even had platform-specific code), making this a -20 net LoC change. Yay!Edit:
Also fixed processing of horizons in dry runs and added a test for that.