bloop icon indicating copy to clipboard operation
bloop copied to clipboard

Support JUnit 5 tests

Open Arthurm1 opened this issue 6 years ago • 16 comments

Currently there is support for JUnit 4 testing in Bloop and SBT by including "com.novocode" % "junit-interface" % "0.11"

This library will run JUnit 5 tests only if they are annotated with @RunWith(JUnitPlatform.class). Gradle directly supports JUnit 5 so non of my projects' tests are marked with this and therefore no classes are recognised as tests.

There is "net.aichler" % "sbt-jupiter-interface" % "0.8.2" which supports JUnit5 without any need for additional annotations but this uses specific code in its SBT plugin (not the Fingerprint implementation) to identify tests and so it doesn't work in Bloop. The reasoning behind this (stated here) is that SBT (and Bloop) doesn't recognise package private test classes. I'm not even sure that checking package private classes would work as JUnit 5 allows you to annotate just class methods and it seems that SBT/Bloop purely check for class annotations only - is that correct?

Can this be changed? So then (hopefully) this library can be changed to then work with Bloop? I haven't spoken to the library author about this so I may have misinterpreted what's going on.

Arthurm1 avatar Jul 31 '19 11:07 Arthurm1

@Arthurm1 Thanks for reporting this ticket, we should definitely find a way to support JUnit 5! I cannot speak to the details of how I imagine this can work now because of lack of time, but I propose that you contact the author and that we have a quick chat in our Gitter channel about this. Porting the code in the sbt plugin to bloop would definitely be welcome, though we should first investigate whether we can fix the issue with package private classes in Zinc's analysis format for the sake of maintenance and speeding up test execution (if the Jupiter test collector is class loading the dependency class path to fix the issue, that'd could also translate in a performance issue).

jvican avatar Aug 07 '19 20:08 jvican

@Arthurm1 Are you interested in contributing this?

jvican avatar Sep 13 '19 17:09 jvican

I've been mulling over how to best to approach it.

JUnit 5 allows various different ways of annotating tests that go beyond what the SBT test-interface can identify e.g.

class StandardTests {

  @Test
  void succeedingTest() {
    // some test
  }
}

So I see 2 options...

  1. Could the SBT test-interface AnnotatedFingerprint interface be changed to allow checks on non-public methods of non-public classes? (I'm leaning towards this)

  2. Or could the whole classpath just be dumped on the test framework to search for test classes (as currently happens in the net.aichler library) - does the performance of searching for tests really matter? It would certainly make test identification simple compared to option 1.

I assume you don't want to bake test identification into Bloop itself?

I'd be interested in contributing if you have more of an idea on a direction to take.

Arthurm1 avatar Sep 13 '19 19:09 Arthurm1

does the performance of searching for tests really matter?

Yes it does.

Could the SBT test-interface AnnotatedFingerprint interface be changed to allow checks on non-public methods of non-public classes? (I'm leaning towards this)

Could you look into this? It does look like the most robust solution and could be useful for other folks. Having support for JUnit 5 would be great.

I assume you don't want to bake test identification into Bloop itself?

Honestly, I wouldn't mind, but I would need to understand better the trade-offs involved. If you get back to me with an analysis of what we would merge in the repository and what it's necessary to bring that logic to the repo (and how much logic there is), then we can study this possibility. Sometimes, it's just nice to have the code inlined in the library and FWIW I don't mind maintaining those API.

jvican avatar Dec 20 '19 19:12 jvican

Some years later, I am having some troubles understanding this (not very familiar with SBT or bloop internals).

  1. I can see that bloop allows you to mark TestFrameworks -- you can do this in SBT with testFrameworks += TestFramework(), or I believe in .bloop/<project-name>.json via the key project.test.frameworks, which looks like:
{
"test": {
    "frameworks": [
        {
            "names": [
                "com.novocode.junit.JUnitFramework"
            ]
        },
        {
            "names": [
                "org.scalatest.tools.Framework",
                "org.scalatest.tools.ScalaTestFramework"
            ]
        },
        // ETC...
    ],
}
  1. So I have tried to install this library, and then manually add it to the frameworks list here:
  • https://github.com/sbt/sbt-jupiter-interface
  1. I can also see from the code in the repo, that the Scala sbt-interface class for TestFramework that should be used is net.aichler.jupiter.api.JupiterFramework
  • https://github.com/sbt/sbt-jupiter-interface/blob/dc737ac36eaacb585eec069b91d2c6520fb983d9/src/plugin/src/main/scala/net/aichler/jupiter/sbt/JupiterPlugin.scala#L36
  1. So I try to add this to <project-name>-test.json in .bloop directory:
"test": {
    "frameworks": [
        {
            "names": [
                "net.aichler.jupiter.api.JupiterFramework"
            ]
        },
   ]
}
  1. Then in Metals, I restart the bloop build server.
package com.example
import org.junit.jupiter.api.Test

class FooTest:
  @Test
  def testFoo(): Unit =
    println("simple.FooTest#testFoo() executed")

But still there is nothing 😢 Why doesn't it work? Maybe I could help to fix it if someone could explain it simply to me =)

Here is a sample project by the way -- it uses Maven and the Maven bloop plugin:

GavinRay97 avatar Aug 12 '21 23:08 GavinRay97

As far as I understand...

To identify tests, SBT (and Bloop) iterate through all the public classes and methods in the project classes dir and query the specified test frameworks to see if these classes/methods are tests relevant to these frameworks.

This is fine for most test frameworks, except for JUnit 5 which, unlike JUnit 4, allows non-public classes/methods to be declared as tests. (At least that is my understanding from this)

All the SBT test interface implementations (JUnit4, munit etc.) are defined as libraries that implement AnnotatedFingerprint or SubclassFingerprint to identify tests, but sbt-jupiter-interface has to be an SBT plugin as it has to have its own specific code to identify non-public tests.

Bloop doesn't understand SBT plugins so can't make use of the plugin and (I think) SBT/Bloop don't query non-public classes/methods so JUnit5 tests can't be identified using the normal SBT test interface.

If you wanted to look at the code (and if I remember correctly), the list of classes to be checked for being tests is created here and queried against the test frameworks here

There could be more to it but I never got time to look further.

Arthurm1 avatar Aug 13 '21 00:08 Arthurm1

@Arthurm1

Ahh -- that makes sense to me, thank you

But there is one thing which is confusing That Scala code above compiles to only public Java methods as bytecode 😧

So if it does not work even as fully public code then I am not sure what might be the root of the issue 🤔

image

GavinRay97 avatar Aug 13 '21 14:08 GavinRay97

You should probably ask the author about it but I don't think the plugin uses the test discovery sbt test interface methods at all.

You can see here that the working Junit 4 implementation tells the build tool to look for the annotation org.junit.Test

You can see here the Junit 5 implementation returns the annotation name net.aichler.jupiter.api.JupiterTestFingerprint which is fake as it's to stop the build tool identifying any tests in the usual way

And here is where the discovery is done and you can see the plugin hands that task over to the JUnit5 library.

Your example above does produce a public class that technically could be discovered using an implementation of AnnotatedFingerprint that told the build tool to look for org.junit.jupiter.api.Test but it would be equally valid in JUnit5 to define that method as protected and then it wouldn't be discovered.

Arthurm1 avatar Aug 14 '21 18:08 Arthurm1

You should probably ask the author about it but I don't think the plugin uses the test discovery sbt test interface methods at all.

You can see here that the working Junit 4 implementation tells the build tool to look for the annotation org.junit.Test

You can see here the Junit 5 implementation returns the annotation name net.aichler.jupiter.api.JupiterTestFingerprint which is fake as it's to stop the build tool identifying any tests in the usual way

And here is where the discovery is done and you can see the plugin hands that task over to the JUnit5 library.

Your example above does produce a public class that technically could be discovered using an implementation of AnnotatedFingerprint that told the build tool to look for org.junit.jupiter.api.Test but it would be equally valid in JUnit5 to define that method as protected and then it wouldn't be discovered.

Ahh sonofabitch, yeah I see what you're saying. I'm still not quite sure I understand the entirety of it -- but I think that if I fork this repo, and modify the contents of:

    /**
     * @return The name of this class. This is to ensure that SBT does not find
     *      any tests so that we can use JUnit Jupiter's test discovery mechanism.
     */
    @Override
    public String annotationName() {

        return getClass().getName();
    }

To instead have:

    @Override
    public String annotationName() {
        return "org.junit.jupiter.api.Test";
    }

Then maybe we'd be getting somewhere? I will try this and report back

GavinRay97 avatar Aug 15 '21 22:08 GavinRay97

~~Would we be just able to use JUnit via LauncherDiscoveryRequestBuilder to discover the tests?~~

Nvm me, I need to read up on the topic more.

tgodzik avatar Aug 17 '21 15:08 tgodzik

Anyway, this looks like we need to implement it entirely in Bloop, perhaps we could even do some custom logic for discovery. Not 100% sure yet how best to try achieve it.

From what I've seen the logic checking if class is public is in Zinc, so it might be harder to change that.

Alternatively, a better overall JUnit 5 support could be a proposal to Scala Center that could be raised by community representatives. This way the priority will be much higher and we could spend some considerable resources on this.

tgodzik avatar Aug 17 '21 15:08 tgodzik

Anyway, this looks like we need to implement it entirely in Bloop, perhaps we could even do some custom logic for discovery

This was along the lines of what I was thinking tbh

  • Currently, I think the reason this doesn't work in bloop is because (quite sensibly so, given state of Scala users preferred tooling) bloop choose to implement sbt:test-agent for detection of test classes
  • Doing this means that you cannot use a regular Java .jar based test library like JUnit, or TestNG, but instead must write an adapter for it using sbt:testing-interfaces
    • The REASON for this interface and writing adapters was so that sbt test and sbt ~test would work universally

But, bloop is quite revolutionary because it divorces from the build tool. As far as I understand, it is a similar idea to IR code in a compiler

  • Bloop IR .bloop/<name>.json files can be used to execute build server actions and provide LSP diagnostics

@tgodzik

Actually, I have an idea. I have been thinking and drafting all day on this. Since bloop is not bound to sbt only, I think there may be a second solution in bloop for entirely generic Test Framework support

Many modern test frameworks offer self-contained .jar's which you can run on command-line with a combination of args to give them the classpath, test names, etc you want to test with.

  • https://junit.org/junit5/docs/current/user-guide/#running-tests-console-launcher
  • https://testng-docs.readthedocs.io/runningtestng/

Here is a rough sketch of my proposal, and sample code of it that would work for JUnit5, and Kotest.

  • The gist of it is a JarRunnableTestFramework, which has access to LSP data about classes and files
  • Any test framework which provides a self-contained .jar that can be run on CLI and given arguments for the classes, to be run, can be modeled as a JarRunnableTestFramework
  • The implementation of methods executeMethodTest(testName: String), executeClassTests(className: String) etc are responsible for the mapping between LSP data and the right set of framework-specific flags to produce proper behavior
  • If the test framework:
    • Artifact requires a non-standard repository, that can be allowed to be specified
    • .jar is not self-contained, and required other .jar's, they can be specified in dependencyJars: Option[Seq[String]] and downloaded during def resolveDependencies()
trait HasLanguageServerDataAvailable:
  def getLspDataForResource(resource: java.io.File) = ???

trait RunnableTestFrameworkParams:
  case class Arg(flags: Seq[String], description: Option[String])
  val artifactId: String
  val version: String
  val cliArgs: Set[Arg]
  // Authors might use a custom artifact repository, or a snapshot repository, so we need to allow to set coordinates so it can be fetched/downloaded
  val repositoryId: Option[String] = Option.empty
  // Ideally the test framework binary should be a self-contained Fat Jar/Uberjar
  // But if not, we need to allow to specify the dependency jars required to have on classpath for it to run
  val dependencyJars: Option[Seq[String]] = Option.empty

trait JarRunnableTestFramework(params: RunnableTestFrameworkParams)
    extends HasLanguageServerDataAvailable:
  lazy val jarName: String = s"${params.artifactId}-${params.version}.jar"
  // Where the magic would happen
  // Bloop/Metals provides CLI arguments, and this invoke the .jar that provides the test executor with the proper set of arguments.
  def executeMethodTest(testName: String) = ???
  def executeClassTests(className: String) = ???
  def executeModuleTests(packageName: String) = ???
  def executeAllTests() = ???
  def resolveDependencies() = ???

class JUnit5RunnableFramework
    extends JarRunnableTestFramework(new RunnableTestFrameworkParams:
      val artifactId = "junit-platform-console-standalone"
      val version = "1.7.2"
      val cliArgs = Set(
        Arg(
          flags = Seq("-cp", "--classpath", "--class-path=PATH[;|:PATH...]"),
          description = Some(
            "Provide additional classpath entries -- for example, for adding engines and their dependencies. This option can be repeated."
          )
        ),
        Arg(
          flags = Seq("-o", "--scan-module=NAME"),
          description = Some(
            "EXPERIMENTAL: Select single module for test discovery. This option can be repeated."
          )
        )
      )
    )

class KotestRunnableFramework
    extends JarRunnableTestFramework(new RunnableTestFrameworkParams:
      val artifactId = "io.kotest.engine.launcher.MainKt"
      val version = "4.6.1"
      val cliArgs = Set(
        Arg(
          flags = Seq("-testpath"),
          description = Some(
            "A path to the test to execute. Nested tests will also be executed"
          )
        ),
        Arg(
          flags = Seq("-packageName"),
          description = Some("restricts tests to the package or subpackages.")
        ),
        Arg(
          flags = Seq("-spec"),
          description = Some(
            "the fully qualified name of the spec class which contains the test to execute"
          )
        )
      )
    )

GavinRay97 avatar Aug 17 '21 21:08 GavinRay97

Can this not be solved by adding something like getMinimumScope method to sbt.testing.Fingerprint and have it return Public, Package Private etc. Then maybe Discovery can be changed to take that minimum scope parameter and check against that instead of allowing only public? Discovery could even be altered without changing Fingerprint but there may be performance implications with that.

I haven't delved too deeply into the sbt test interface, but is there a fundamental issue with it that means a complete redesign? There are already implementations for JUnit 3 & 4, MUnit, ScalaTest, TestNG, Specs2, uTest, miniTest (maybe more) and a number of build tools use the interface to offload testing: Bloop, SBT, mill, seed, fury(?).

Besides test discovery, the SBT test interface also manages transforming all the test results back into the same form that the build tool/server can understand and I think it takes care of test selection/filtering.

Arthurm1 avatar Aug 18 '21 10:08 Arthurm1

Can this not be solved by adding something like getMinimumScope method to sbt.testing.Fingerprint and have it return Public, Package Private etc. Then maybe Discovery can be changed to take that minimum scope parameter and check against that instead of allowing only public? Discovery could even be altered without changing Fingerprint but there may be performance implications with that.

It might be better to change the interface instead of coming up with an alternative solution, which would allow all other tools to work with it. We could move the discussion to sbt repo to see if they are against that and why it was never done?

tgodzik avatar Aug 18 '21 13:08 tgodzik

It seems like I've been hit by the same root problem using ScalaTest.

As detailed in (https://github.com/scalameta/metals/discussions/4016), we're writing all of our ScalaTest test suites as private classes.

The tests are discovered by Maven (our current build tool) and Idea respectively. On VSCode with Metals, which uses Bloop, the tests are not being discovered.

I would very much appreciate it if Bloop would behave like the other build tools/the Idea Scala plugin!

Edit: Take-away: It's not just a JUnit 5 issue - depending on the individual code style ScalaTest is hit as well!

LeUser111 avatar Jun 10 '22 11:06 LeUser111

Just checked in sbt and we can't run private classes either, so probably it's the same root cause.

tgodzik avatar Jun 10 '22 18:06 tgodzik