Skip to content
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 support for JUnit5 and provide a solution for JUnitRunner as an Extension #1454

Closed
carlspring opened this issue Dec 19, 2018 · 49 comments
Closed

Comments

@carlspring
Copy link

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
Copy link

adarro commented Jul 11, 2019

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

  • org.junit.jupiter:junit-jupiter-api
  • org.junit.jupiter:junit-jupiter-engine
  • org.junit.jupiter:junit-jupiter-junit-platform-runner

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
Copy link

mfulgo commented Feb 10, 2020

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
Copy link

giurim commented Mar 7, 2020

@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
Copy link

gna-phetsarath commented Dec 2, 2022

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

@cheeseng
Copy link
Contributor

cheeseng commented Dec 6, 2022

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

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

Cheers.

@dragos
Copy link

dragos commented May 26, 2023

@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
Copy link
Contributor

@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
Copy link

dragos commented May 30, 2023

@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
Copy link

giurim commented May 30, 2023

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
Copy link

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
Copy link
Contributor

@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
Copy link
Contributor

@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
Copy link
Contributor

cheeseng commented Jun 9, 2023

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

  1. Create a new repo under scalatest call scalatestplus-junit5.
  2. Ported the PR JUnit 5 Support scalatestplus-junit#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
Copy link
Contributor

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

@marcphilipp
Copy link

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
Copy link
Contributor

@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
Copy link
Contributor

@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
Copy link

@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
Copy link

dragos commented Jun 20, 2023

@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
Copy link

@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
Copy link
Contributor

@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
Copy link

dragos commented Jun 20, 2023

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

@cheeseng
Copy link
Contributor

@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
Copy link
Contributor

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

scalatest/scalatestplus-junit5@d257ba9

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

Thanks.

@marcphilipp
Copy link

@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 added a commit to scalatest/scalatestplus-junit5 that referenced this issue Jun 21, 2023
…erride mayRegisterTests = true in ScalaTestClassDescriptor instead, as suggested by @marcphilipp here scalatest/scalatest#1454 (comment) .
@cheeseng
Copy link
Contributor

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
Copy link
Contributor

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

Cheers.

@cheeseng
Copy link
Contributor

@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.

@cheeseng
Copy link
Contributor

@marcphilipp Thanks!

@cheeseng
Copy link
Contributor

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

scalatest/scalatestplus-junit5@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
Copy link
Contributor

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

scalatest/scalatestplus-junit5#2

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

Cheers.

@cheeseng
Copy link
Contributor

cheeseng commented Jul 5, 2023

@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
Copy link

dragos commented Jul 7, 2023

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
Copy link
Contributor

cheeseng commented Jul 7, 2023

@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
Copy link
Contributor

cheeseng commented Jul 8, 2023

@marcphilipp @dragos I added the getSource initial implementation:

scalatest/scalatestplus-junit5@71d0f85

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
Copy link

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

@cheeseng
Copy link
Contributor

@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
Copy link
Contributor

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

@cheeseng
Copy link
Contributor

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

scalatest/scalatestplus-junit5#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
Copy link
Contributor

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

Cheers.

@cheeseng
Copy link
Contributor

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

@marcphilipp
Copy link

@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
Copy link
Contributor

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

@marcphilipp
Copy link

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

@cheeseng
Copy link
Contributor

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

@cheeseng
Copy link
Contributor

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

@carlspring
Copy link
Author

carlspring commented Jul 25, 2023

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
Copy link
Contributor

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants