Increase Fidelity of the sbt.testing.Framework Implementation
- when
Selectors supplied include onlyTestSelectors andTestWildcardSelectors, filter properties to run by matching their names against theSelectors; - added a test that actually runs ScalaCheck via the
SBT Test Interfaceand demonstrates the (now correct) treatment of theSelectors; - test also demonstrates two unfixable infidelities in the treatment of the nested properties;
- thankfully, ScalaCheck's implementation of
sbt.testing.Frameworkis, unlike in some other test frameworks, shared between the platforms (JVM, Scala.js, Scala Native), so the fixes do not have to be replicated for each platform, but: - the test needs to supply a
testClassLoader: ClassLoaderparameter when callingsbt.testing.Framework.runner(); on platforms other than JVM,getClass.getClassLoaderis not available, soPlatform.getClassLoader: ClassLoadermethod was added to everyPlatform; on platforms other than JVM, it returnsnull- which is fine, since on those platformssbt.testing.Framework.runner()ignores thetestClassLoaderparameter anyway.
fixes #1105
@satorg any thoughts?
@satorg I am starting to suspect that the plan here is to let this fix wither on the wine like the previous contribution in this area (https://github.com/typelevel/scalacheck/pull/1031); if this is true, and my contribution is for some reason not wanted, please say so; otherwise - what do I need to do to get this merged?
Thank you!
@dubinsky thank you for the PR and sorry for the long reply!
Personally, I'd love to get both PRs (this and #1031) merged ultimately. I just don't feel very confident about this ScalaCheckFramework implementation, because I personally don't use it and mostly rely on other test framework runners.
Do you think your PR complements #1031 or supersedes it completely?
Personally, I'd love to get both PRs (this and #1031) merged ultimately. I just don't feel very confident about this ScalaCheckFramework implementation, because I personally don't use it and mostly rely on other test framework runners.
Code changes have comments, tests are added that verify the change, and issue #1105 has detailed explanations. Is there anything else I can do? Are you going to involve other contributors/maintainers that are more familiar with this area or do you have specific questions?
Do you think your PR complements #1031 or supersedes it completely?
My pull request completely supersedes #1031: it handles both TestSelector and TestWildcardSelector, whereas #1031 handles just TestSelector.
Thank you!
@dubinsky , thank you for your work on this issue. I found some time to wrap my head around SBT testing API, so that now I think understand what is going on there to some extent. If you are still interested, I guess we can get your PR merged.
However, I wouldn't like to dismiss the effort made in #1031 too, since it was kinda preceding yours. How about if I merge the former PR first, then you'll update yours overwriting conflicting parts and we'll merge your PR right after? Thereby everyone will get involved. @Duhemm, in a case you are still around, let us know if you are OK with this plan please. Thanks everyone!
@dubinsky , thank you for your work on this issue.
My pleasure!
If you are still interested, I guess we can get your PR merged.
Yes.
However, I wouldn't like to dismiss the effort made in #1031 too, since it was kinda preceding yours. How about if I merge the former PR first, then you'll update yours overwriting conflicting parts and we'll merge your PR right after?
Of course! Thank you!
@dubinsky , do you have any clue what serializeTask/deserializeTask methods of ScalaCheckRunner could be for? They look like they are supposed to be called via reflection, but I didn't manage to figure out when and how exactly.
@dubinsky , do you have any clue what
serializeTask/deserializeTaskmethods ofScalaCheckRunnercould be for? They look like they are supposed to be called via reflection, but I didn't manage to figure out when and how exactly.
They are called by the Test Interface/Test Bridge on Scala.js and Scala Native to transport the task information from JVM to Node.js/Native code.
Hi @dubinsky , I've merged #1031. Feel free to update this one and let's get it merged too. Thanks!
update this one and let's get it merged too. Thanks!
Done. Thank you!