-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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(fuzzer): remove verifyWindow() in AggregationFuzzer #12391
base: main
Are you sure you want to change the base?
Conversation
Summary: We used to test window operations in aggregaiton fuzzer before the window fuzzer was built. Since we now have a window fuzzer that has much better coverage, remove the verifyWindow method in AggregationFuzzer. Differential Revision: D69886065
This pull request was exported from Phabricator. Differential Revision: D69886065 |
✅ Deploy Preview for meta-velox canceled.
|
…ubator#12391) Summary: We used to test window operations in aggregaiton fuzzer before the window fuzzer was built. Since we now have a window fuzzer that has much better coverage, remove the verifyWindow method in AggregationFuzzer. Differential Revision: D69886065
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
// 10% of times test window operator. | ||
const bool sortedInputs = FLAGS_enable_sorted_aggregations && | ||
canSortInputs(signature) && vectorFuzzer_.coinToss(0.2); | ||
|
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.
Why was the removal of 10% window fuzzer replaced by 0.2 coin tosses ?
…ubator#12391) Summary: We used to test window operations in aggregaiton fuzzer before the window fuzzer was built. Since we now have a window fuzzer that has much better coverage, remove the verifyWindow method in AggregationFuzzer. Reviewed By: natashasehgal Differential Revision: D69886065
Summary:
We used to test window operations in aggregaiton fuzzer before the window fuzzer was built. Since
we now have a window fuzzer that has much better coverage, remove the verifyWindow method in
AggregationFuzzer.
Differential Revision: D69886065