scalacenter / bloop

Bloop is a build server and CLI tool to compile, test and run Scala fast from any editor or build tool.
https://scalacenter.github.io/bloop/
Apache License 2.0
909 stars 201 forks source link

Support JUnit 5 tests #996

Open Arthurm1 opened 5 years ago

Arthurm1 commented 5 years ago

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.

jvican commented 5 years ago

@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 commented 5 years ago

@Arthurm1 Are you interested in contributing this?

Arthurm1 commented 5 years ago

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.

jvican commented 4 years ago

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.

GavinRay97 commented 3 years ago

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:

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

  3. So I try to add this to <project-name>-test.json in .bloop directory:

    "test": {
    "frameworks": [
        {
            "names": [
                "net.aichler.jupiter.api.JupiterFramework"
            ]
        },
    ]
    }
  4. 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:

Arthurm1 commented 3 years ago

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.

GavinRay97 commented 3 years ago

@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

Arthurm1 commented 3 years ago

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.

GavinRay97 commented 3 years ago

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

tgodzik commented 3 years ago

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 commented 3 years ago

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.

GavinRay97 commented 3 years ago

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

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


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

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

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"
          )
        )
      )
    )
Arthurm1 commented 3 years ago

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.

tgodzik commented 3 years ago

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?

LeUser111 commented 2 years ago

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!

tgodzik commented 2 years ago

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