-
Notifications
You must be signed in to change notification settings - Fork 407
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
Verify only props matching the given TestSelectors #1031
base: main
Are you sure you want to change the base?
Conversation
46dbc34
to
7d9e2a9
Compare
Previously, ScalaCheck ignored the selectors that it receives as input in the "root task". This prevented users from running only a subset of properties in a specification by passing their `TestSelector` to ScalaCheck in a `TaskDef`. This patch fixes this by having the root task program the execution of only the requested properties if the `TaskDef` contains only `TestSelector`s. ScalaCheck's behavior remains unchanged if the `TaskDef` contains any other kind of `Selector`.
Hi @satorg! Would you mind taking a look at this PR, please? |
@satorg Hi! Any concerns with this PR? |
Hi @Duhemm ! Thank you for the ping. I'm really sorry that it takes so much time to review your PR. Honestly, I made a couple of attempts to look into it, but every time was realizing that I'm not really sure what the problem the PR is aimed to solve. It does not mean that there's anything wrong with your PR, but rather it seems that I've never encountered that kind of issue in my personal experience. So I would really appreciate if you could elaborate a bit more on the issue the PR is addressing, maybe with some really small examples. I believe it could help to move it forward. Anyway, thank you for contributing to the project! |
Hi @satorg, thanks for taking a look and sorry for taking so long to get back to you! sbt's test-interface API has a concept of For instance, Bloop uses In my case, I am working on a plugin that adds test retry in sbt. When a test case ("property", in ScalaCheck) fails, it is automatically retried a few times until it passes so that the build is not blocked by flaky tests. The changes in this PR will make it possible for the plugin to retry only the failing test case instead of the entire test object. Please let me know if you have other questions or if the benefits of this PR are still unclear. |
Hi @satorg! Was my message above helpful or do you have other questions and concerns? |
core/jvm/src/test/scala/org/scalacheck/ScalaCheckFrameworkSpecification.scala
Outdated
Show resolved
Hide resolved
Previously, only the last property would be verified and the others would be ignored. This patch fixes the tests so that all properties are verified.
@Duhemm , thank you for the clarification and sorry again for the long review. I'm really trying to wrap my head around the problem. You mentioned the
I.e. only the properties with names containing "BigInt" get executed. Is there any way to reproduce the issue you're targeting with any existing testing framework or is it specific to the plugin you're working on? |
Previously, ScalaCheck ignored the selectors that it receives as input in the "root task". This prevented users from running only a subset of properties in a specification by passing their
TestSelector
to ScalaCheck in aTaskDef
.This patch fixes this by having the root task program the execution of only the requested properties if the
TaskDef
contains onlyTestSelector
s. ScalaCheck's behavior remains unchanged if theTaskDef
contains any other kind ofSelector
.