scalatest / scalatest

A testing tool for Scala and Java developers
Apache License 2.0
1.15k stars 333 forks source link

Add support for JUnit5 and provide a solution for JUnitRunner as an Extension #1454

Closed carlspring closed 1 year ago

carlspring commented 5 years ago

In JUnit5 the @RunWith annotated classes need to now use the @ExtendWith annotation, which, in term, is no longer using a Runner, but and Extension. Hence, the code will have to be re-worked a bit in order to support JUnit5.

adarro commented 5 years ago

As a stopgap solution, you can run your existing ScalaTest using the Platform engine by replacing the JUnit Runner class with JUnitPlatform class.

i.e. replace import: org.scalatest.junit.JUnitRunner with: org.junit.platform.runner.JUnitPlatform

replacing the annotation @RunWith(classOf[JUnitRunner]) becomes @RunWith(classOf[JUnitPlatform])

replace Junit4 dependency in your [sbt / Gradle / Maven] build script with

Junit4

`package io.truthencode.example.junit4

import org.scalatest.junit.JUnitRunner import org.junit.runner.RunWith import org.scalatest.{FunSpec, Matchers}

@RunWith(classOf[JUnitRunner]) class JUnitExampleTest extends FunSpec with Matchers { describe("ScalaTest with JUnit4") {

it("should pass this test") {
 1 should equal(2 - 1)
}

it("should fail here") {
  1 should equal(45)
}

} }`

Becomes

JUnit 5

`package io.truthencode.example.junit5

import org.junit.platform.runner.JUnitPlatform import org.junit.runner.RunWith import org.scalatest.{FunSpec, Matchers}

@RunWith(classOf[JUnitPlatform]) class JUnitExampleTest extends FunSpec with Matchers { describe("ScalaTest with JUnit5") {

it("should pass this test") {
 1 should equal(2 - 1)
}

it("should fail here") {
  1 should equal(45)
}

} }`

YMMV based on build tool and IDE. I tend to use Gradle in IDEA and have noticed the Test results window shows 'classMethod' instead of the Descriptions Using FunSpc but at least once again captures the results and I can include JUnit 5, ScalaTest and Concordion Acceptance tests in the same build. image

I would at some point love to have my ScalaTests take advantage of JUnit5 Extensions.

mfulgo commented 4 years ago

In contrast to @adarro, attempting to use JUnitPlatform didn't work when running tests from gradle. I had some success with this by using the junit-vintage runner and keeping the @RunWith(classOf[JUnitRunner]).

Digging into it a bit, I think the most correct thing to do is to create a TestEngine for Scalatest, rather than an Extension. See https://junit.org/junit5/docs/current/user-guide/#launcher-api. It would probably go into a scalatestplus library, given the modular design of 3.x.

giurim commented 4 years ago

@mfulgo, In the meantime you can use scalatest-junit-runner which is a simple TestEngine implementation to run Scalatest suites with the JUnitPlatform in Gradle.

gna-phetsarath commented 1 year ago

Is there anyone working/planning on creating a TestEngine for Scalatest?

cheeseng commented 1 year ago

@gna-phetsarath fyi i am making some progress in this branch:

https://github.com/scalatest/scalatestplus-junit/tree/feature-junit-5-support

Cheers.

dragos commented 1 year ago

@cheeseng I've been using https://github.com/helmethair-co/scalatest-junit-runner with good results. However, it took a loooong time to figure all out, with many detours and dead ends along the way. Gradle + ScalaTest + IntelliJ doesn't work well at all today (out of the box). The scalatest-junit-runner project fixes it, so would there be a way to support it by default in ScalaTest? cc @giurim in case you're interested in removing some friction and contributing this to scalatest?

cheeseng commented 1 year ago

@cheeseng I've been using https://github.com/helmethair-co/scalatest-junit-runner with good results. However, it took a loooong time to figure all out, with many detours and dead ends along the way. Gradle + ScalaTest + IntelliJ doesn't work well at all today (out of the box). The scalatest-junit-runner project fixes it, so would there be a way to support it by default in ScalaTest? cc @giurim in case you're interested in removing some friction and contributing this to scalatest?

Good to hear that scalatest-junit-runner works for you, as mentioned in earlier comment scalatestplus-junit has a branch for JUnit 5 too, but I am not sure what's next step, perhaps @giurim can share some experience with us?

dragos commented 1 year ago

@cheeseng what is the status of your branch? If that line of work will support scalatest out of the box in Gradle+IDEA, it would be great, too. Just trying to make the setup more discoverable and simpler for this use case. If it makes sense to base it on @giurim's work, great, but if your line of work is also making progress and has a good chance of landing in a release that's also good. I can't judge what approach is better.

giurim commented 1 year ago

Hi @cheeseng and @dragos,

I am happy if my project can help you to solve problems. AFAIK IDEA also uses an internal test engine to execute Scalatest tests, similar to the one I wrote. I created the tool to bridge the gap until Scalatest "natively" support Junit5.

There are minor differences between the two test systems, so there will be a couple of corener-cases to figure out. Also in some cases the scalatest-junit-runner cannot provide feature parity with native Scalatest because certain level of control or information is not available on the interface I use to interact with Scalatest. As I tend to use some internal classes my solution also not 100% future proof, but exposing certain functionality on a "public" api in Scalatest could help.

I take inspiration for the engine from the kotlintest engine, but thanks to users many bugs were fixed since.

Also I wrote the engine in vanilla java mainly to see if it is possible, so my code is scala-version agnostic. But this approach leads some weird/ugly code on the scala-interop side.

I would be more than happy if my work (or I) would help to solve this issue, please let me know how can I help.

marcphilipp commented 1 year ago

There are minor differences between the two test systems, so there will be a couple of corener-cases to figure out. Also in some cases the scalatest-junit-runner cannot provide feature parity with native Scalatest because certain level of control or information is not available on the interface I use to interact with Scalatest.

If there's anything you're missing from the JUnit Platform API that would help here, please let me know. I'd be happy to help.

cheeseng commented 1 year ago

@giurim @marcphilipp Thanks for sharing and the offer to help, I think it will be really great if we can get these working together. I'll have to refresh myself first on the status of work in progress branch I have, and I'll update the status here again soon!

cheeseng commented 1 year ago

@dragos @giurim @marcphilipp What I have done in my WIP branch was implemented a JUnit5 execution engine, and added AssertionsForJUnit5Macro, AssertionsForJUnit5, JUnit5Suite and JUnit5SuiteLike , the aim was to provide JUnit 5 similar support like what JUnit 4 has. I am not sure what will be the right next step, perhaps I shall look into scalatest-junit-runner to study what it supports, do feel free to share your thoughts on what will be a good next step for us. :)

cheeseng commented 1 year ago

@bvenners @dragos @giurim @marcphilipp Here's my propose plan:

  1. Create a new repo under scalatest call scalatestplus-junit5.
  2. Ported the PR https://github.com/scalatest/scalatestplus-junit/pull/27 into scalatestplus-junit5 .
  3. Publish a M1 version.
  4. Port over examples from scalatest-junit-runner to make sure they works with M1, and port needed things from scalatest-junit-runner to the new scalatestplus-junit5 as needed, this step I think we need many hands to help.

For starting JUnit 5 support with new scalatestplus-junit5 we'll need green light from @bvenners , feel free to share your thought on the plan.

Cheers.

bvenners commented 1 year ago

@cheeseng Sure, doing a separate scalatestplus-junit5 sounds like a good plan.

marcphilipp commented 1 year ago

One minor thing: "scalatest-junit-runner" is a misleading name since "runner" is a concept from JUnit 4. I'm fine with scalatestplus-junit5 but the implementation class should probably be called ScalaTestEngine since it will implement TestEngine.

cheeseng commented 1 year ago

@bvenners @giurim @marcphilipp @dragos fyi I reach step 4 in the plan, the source is available at:

https://github.com/scalatest/scalatestplus-junit5

and the current version is published as 3.2.16.0-M1, we need helping hands to try it out and see if it works / what does not work for you, we'll continue to work from here.

I have also ported over the examples from scalatest-junit-runner:

https://github.com/scalatest/scalatestplus-junit5/tree/main/examples

Cheers.

cheeseng commented 1 year ago

@marcphilipp When I tried in maven-example, I noticed that the discovery code seems to run multiple times when I do mvn clean test, any idea what is causing it?

marcphilipp commented 1 year ago

@cheeseng I think that's just due to the way Maven Surefire is using the Launcher API. It should have a different discovery request with different selectors each time. Is that problematic?

dragos commented 1 year ago

@bvenners @giurim @marcphilipp @dragos fyi I reach step 4 in the plan, the source is available at:

https://github.com/scalatest/scalatestplus-junit5

and the current version is published as 3.2.16.0-M1, we need helping hands to try it out and see if it works / what does not work for you, we'll continue to work from here.

This is great progress! Sorry for missing this, there has been a few busy days here. I will give it a try on our code base today and report back.

marcphilipp commented 1 year ago

@dragos and I tried the engine and ran into an issue with Gradle Enterprise Test Distribution and Predictive Test Selection. In accordance with JUnit's documented requirements, test engines must be able to handle UniqueIdSelector for unique IDs they have previously returned. For example, a unique ID like [engine:scalatest]/[class:com.acme.FooTest] should be converted into the corresponding ScalaTestClassDescriptor in the implementation of TestEngine.discover.

@cheeseng Would this be something you could add?

cheeseng commented 1 year ago

@dragos Thanks for helping to test, do let us know if you encounter any problem.

@marcphilipp Yes I would love to, though I am not sure how to do that yet at the moment, I'll try.

dragos commented 1 year ago

@cheeseng I encountered the problem @marcphilipp described above, since our build uses "predictive test selection".

cheeseng commented 1 year ago

@dragos @marcphilipp Let me work on supporting it, one thing is I never use "predictive test selection" before, would it be possible if you can help provide me a minimal sample maven/gradle project that can reproduce the problem?

cheeseng commented 1 year ago

@dragos @marcphilipp fyi I come out with a first version to support UniqueIdSelector:

https://github.com/scalatest/scalatestplus-junit5/commit/d257ba98a3ca17a6ec71ca9da570432dd93b4d44

I am not sure how I can test it easily, perhaps you have a better idea?

Thanks.

marcphilipp commented 1 year ago

@cheeseng I took a quick look at the code and I think it looks like it should work. If you merge it and release an M2 version, we'd be happy to test it.

One general thing I noticed is that the engine currently doesn't handle redundant TestSelectors. For example, it would be valid to pass a PackageSelector and a ClassSelector for a class in the same package; or, a ClassSelector for a class and a UniqueIdSelector for the same class. JUnit provides a base implementation to deal with such scenarios: EngineDiscoveryRequestResolver. The documentation is a bit sparse but I can recommend looking to the implementations in JUnit Jupiter in Spock for inspiration.

The other thing I wasn't sure about are these scalatest-all-tests descriptors. I assume these are for preventing the ScalaTestClassDescriptor from being pruned by JUnit? If so, you can override mayRegisterTests() to return true instead.

cheeseng commented 1 year ago

The other thing I wasn't sure about are these scalatest-all-tests descriptors. I assume these are for preventing the ScalaTestClassDescriptor from being pruned by JUnit? If so, you can override mayRegisterTests() to return true instead.

Yes, mayRegisterTests works great, thanks for the pointer! I have pushed a commit and shall release M2 with it.

One general thing I noticed is that the engine currently doesn't handle redundant TestSelectors. For example, it would be valid to pass a PackageSelector and a ClassSelector for a class in the same package; or, a ClassSelector for a class and a UniqueIdSelector for the same class. JUnit provides a base implementation to deal with such scenarios: EngineDiscoveryRequestResolver. The documentation is a bit sparse but I can recommend looking to the implementations in JUnit Jupiter in Spock for inspiration.

I'll look into that in my next step, hopefully it is not too hard! :)

cheeseng commented 1 year ago

@marcphilipp @dragos @bvenners Fyi I released M2. I'll continue to EngineDiscoveryRequestResolver as pointed out by @marcphilipp .

Cheers.

cheeseng commented 1 year ago

@marcphilipp Do you mind to share me URL of implementations in JUnit Jupiter in Spock? I tried to find it in https://github.com/xvik/spock-junit5 but didn't find anything related to EngineDiscoveryRequestResolver, I probably looking at the wrong place.

marcphilipp commented 1 year ago

Sure thing!

JUnit Jupiter: https://github.com/junit-team/junit5/blob/4cadc8aa6c9560ac6a63c79f6866facb687b4c9d/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/DiscoverySelectorResolver.java#L35-L42 Spock: https://github.com/spockframework/spock/blob/e08fe5c31c90b99735ae9bb3c25fa5888c778352/spock-core/src/main/java/org/spockframework/runtime/SpockEngine.java#L23-L28

cheeseng commented 1 year ago

@marcphilipp Thanks!

cheeseng commented 1 year ago

@marcphilipp I tried to work out a version of ScalaTestEngine that uses EngineDiscoveryRequestResolver :

https://github.com/scalatest/scalatestplus-junit5/compare/main...feature-engine-discovery-request-resolver

Do you mind to help sanity check the code to see if they looks right? It is still a bit messy with code to convert between java/scala conversion, but hopefully the direction is correct.

cheeseng commented 1 year ago

@bvenners @marcphilipp @dragos Fyi I have released M3 that includes changes in the following PR:

https://github.com/scalatest/scalatestplus-junit5/pull/2

Please kindly help to try and see if it works for you?

Cheers.

cheeseng commented 1 year ago

@marcphilipp @dragos Just a friendly ping, any chance that you may kindly help checking if M3 is working for you?

Thanks for your help!

dragos commented 1 year ago

Sorry for the delay. I had another try with M3 and things look good. I tried both CLI and IntelliJ, and both work. I noticed that the CLI has additional output for successful tests, but I think that's ok. It might be a bit better formatted to make it prettier, but the functionality is there.

com.gradle.enterprise.sbt.internal.configuration.GeConfigHelperTest > system properties override existing tags, links and custom values PASSED

com.gradle.enterprise.sbt.internal.scan.BuildScanListenerTest > findRootCause find root case in simple linked list PASSED
....

Inside IDEA, I was expecting I could navigate to the test case, but that doesn't seem to be supported. That would be a nice addition, but that doesn't work with scalatest-junit-runner either.

Screenshot 2023-07-07 at 10 34 00
cheeseng commented 1 year ago

@dragos Thanks for helping to test! For the test case navigation I think I may need to set the TestDescriptor's source, I am not sure how yet, but I'll look into it.

cheeseng commented 1 year ago

@marcphilipp @dragos I added the getSource initial implementation:

https://github.com/scalatest/scalatestplus-junit5/pull/2/commits/71d0f85d3813a0886e62200f73e4cee84fb6f207

It is released as M4, when I tried it with example projects we have, it seems to work with the maven project but not gradle project, @marcphilipp any idea what's the reason behind it?

marcphilipp commented 1 year ago

@cheeseng How did you run the tests from IntelliJ IDEA? Using the Gradle or IntelliJ test runner?

cheeseng commented 1 year ago

@marcphilipp Oh yes, I was using the gradle runner which is the default runner for gradle project, after I changed it to Intellij runner it starts to work! Thanks for pointing it out!

cheeseng commented 1 year ago

I think we'll have to add that as a note in the README.md, I'll do that.

cheeseng commented 1 year ago

@bvenners @marcphilipp @dragos @giurim Fyi I have added support for tags in the following PR:

https://github.com/scalatest/scalatestplus-junit5/pull/3

and released it as M6. I am thinking we are getting near to a real release, perhaps some tidying up of scaladocs and some quick documentations in the README.md. Do you think there's still important feature that is missing?

cheeseng commented 1 year ago

@bvenners @marcphilipp @dragos @giurim Just fyi I released M7, which tagging at the test level support is added also.

Cheers.

cheeseng commented 1 year ago

@adarro @marcphilipp @dragos @giurim @adarro @carlspring @gna-phetsarath I am planning to release 3.2.16.0 final soon, any objection?

marcphilipp commented 1 year ago

@cheeseng No objections from my side. I wanted to test it more thoroughly and think I should be able to do so until mid next week if you're willing to wait so long.

cheeseng commented 1 year ago

@marcphilipp Mid next week is fine, let's do that. I am preparing things in scalatest.org website to be ready.

marcphilipp commented 1 year ago

@cheeseng I finally had a chance to run our integration tests against it and it looks good! 👍

cheeseng commented 1 year ago

@marcphilipp Glad to hear that! Thanks a lot for helping to test! 🙏🏻

cheeseng commented 1 year ago

@adarro @marcphilipp @dragos @giurim @adarro @carlspring @gna-phetsarath I'll arrange to release 3.2.16.0 final soon, thanks! 🙏🏻

carlspring commented 1 year ago

Thanks for all your hard work! I am no longer working at the company which had one of the world's largest Scala code bases, so I won't be able to test this myself (as that was about five years ago).

However, I'm glad to hear this has now been implemented! I'm sure it'll help a lot of developers.

cheeseng commented 1 year ago

@adarro @marcphilipp @dragos @giurim @adarro @carlspring @gna-phetsarath just fyi, final version of 3.2.16.0 is released. I'll close this ticket.

Thank you everyone for making it happen!

Cheers.