-
-
Notifications
You must be signed in to change notification settings - Fork 97
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 partition operator to filter expressions? #159
Comments
cc @Guiguiprim if you wanted to tackle this. |
How would you see this ? I never used partitioning, so I don't know what would be the right API for it.
Would this, be a scenario we want to allow:
|
For a far, it seems to me that the numeric ID strategy might not always works. Worst case the filter happen to eliminate all the odd tests...? |
I was just thinking of adding See https://nexte.st/book/partitioning.html for how partitioning works. |
I think we should let people do this if we like -- it's up to them to exhaustively run all the tests in complicated situations. The more common approach will be to simply do
Hmm, I'm not sure what you mean. From my view the point of partitioning is to not run the tests that aren't in the right bucket -- this seems like a good use case for filter expressions. |
I mean:
Am I wrong ? That's also why I saw it like It's a syntax choice I guess: |
Use an index maintained by the caller rather than state maintained by the callee -- this removes mutable state and also makes it easier to implement a partition() operator in filter expressions (#159). A TODO is to remove TestFilterBuilder and PartitionerBuilder, neither of which are required any more.
This reverts commit 14d7791. As @Guiguiprim pointed out in #159, we want the partition operator to apply after others.
Ahhh yes you're absolutely right. Sorry about misunderstanding you earlier -- you're absolutely right that partition operations must happen after all other filters are applied. I think we shouldn't implement a |
… end As @Guiguiprim pointed out in #159, partition-based filtering must happen after all other kinds.
I added a comment to try and explain this -- hopefully that'll prevent future me from being confused! |
I guess we could support hash-based partitioning though, maybe at some point in the future. |
Consider adding a
partition()
operator to filter expressions, and possibly deprecating the current--partition
flag.This is in principle possible to do, since (assuming a specific state of source code) all partition assignments for tests are completely deterministic. However, the current partition scheme for
count:
uses mutable state, which is too complicated -- instead, we can simply give each test a numeric ID and simply use modulo N for thecount
partition operator.Strictly speaking, this doesn't block the release of filter expressions. However, if we do it before releasing filter expressions, we can ban using
--partition
and-E
simultaneously. If we implement this after the release, we'll have to scan the query to look forpartition
operators, raising the complexity of the implementation -- so there's some advantages to doing it now.The text was updated successfully, but these errors were encountered: