Increase currying test coverage (#2894) #2939
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@lgtout would it be possible to comment out the code which doesn't work because of the required Kotest changes, so we can merge the one that works and then put the commented out one back when Kotest is updated? |
|
@serras Yes, I can modify this branch to have just the tests that don't require the Kotest update. |
|
@lgtout I've move forward and commented out the tests myself. Is there something else you want in this PR, or could be mark it as ready for review? |
|
Thank you @serras . No, nothing else. Can be marked as ready-for-review. |
serras
approved these changes
Mar 31, 2023
nomisRev
approved these changes
Apr 3, 2023
|
Thank you @lgtout! 🙌 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR achieves 100% test coverage for
curried,uncurried,curriedEffect, anduncurriedEffect.Arity-22
uncurriedEffectwas missing, so I added it and updatedarrow-core.api.Currently Kotest's
checkAll,forAll,forNonedo not support tests with more than 12 generators.As such, I had to submit a PR to Kotest to support up to 22 generators in order to use property testing on Arrow's curry functions.
The Kotest PR has been merged into
masterand is due to ship in the next release (5.6.0). But it's unclear when that will be.I was able to use the Kotest update by adding the snapshot repository address
https://s01.oss.sonatype.org/content/repositories/snapshots/and setting Kotest version to5.6.0.1188-SNAPSHOT. But I didn't include those build changes in this PR.As such, this PR will fail the Arrow pipeline. Other than that, the PR is complete.
Related: closed draft PR to get feedback on general direction of changes.