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

No tests were found when trying to run JUnit tests with Scala 2 or WIP Scala 3 extension #18373

Closed
GavinRay97 opened this issue Jul 3, 2021 · 6 comments
Labels
env/windows Impacts Windows machines kind/bug Something isn't working triage/invalid This doesn't seem right

Comments

@GavinRay97
Copy link
Contributor

GavinRay97 commented Jul 3, 2021

Zulip thread: https://quarkusio.zulipchat.com/#narrow/stream/187038-dev/topic/.22No.20Tests.20Found.22.2C.20last.20issue.20for.20Scala.203.20support

Describe the bug

  • Go to https://code.quarkus.io/ and create a new Maven application with just the Scala extension

  • Try to run mvn test or mvn quarkus:test

  • The maven-scala-plugin will detect the presence of the Scala test files and compile them, but Quarkus/JUnit 5 will report there being no tests.

  • image

  • image

  • image


I believe this may be something specific to the way Quarkus deals with tests. I thought it might be a JUnit bug, and was going to file an issue there, but I downloaded their junit5-jupiter-maven starter, and tried to reproduce this, but had no issues:

The pom.xml being used here is:

<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
		 xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
	<modelVersion>4.0.0</modelVersion>

	<groupId>com.example</groupId>
	<artifactId>junit5-jupiter-starter-maven</artifactId>
	<version>1.0-SNAPSHOT</version>

	<properties>
		<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
		<maven.compiler.source>1.8</maven.compiler.source>
		<maven.compiler.target>${maven.compiler.source}</maven.compiler.target>

		<surefire-plugin.version>3.0.0-M5</surefire-plugin.version>

		<scala-maven-plugin.version>4.5.3</scala-maven-plugin.version>
		<scala.compat.version>3</scala.compat.version>
		<scala.version>3.0.0</scala.version>
	</properties>

	<dependencyManagement>
		<dependencies>
			<dependency>
				<groupId>org.junit</groupId>
				<artifactId>junit-bom</artifactId>
				<version>5.7.2</version>
				<type>pom</type>
				<scope>import</scope>
			</dependency>
		</dependencies>
	</dependencyManagement>

	<dependencies>
		<dependency>
			<groupId>org.junit.jupiter</groupId>
			<artifactId>junit-jupiter</artifactId>
			<scope>test</scope>
		</dependency>
		<dependency>
			<groupId>org.scala-lang</groupId>
			<artifactId>scala3-library_${scala.compat.version}</artifactId>
			<version>${scala.version}</version>
		</dependency>
		<dependency>
			<groupId>org.scala-lang</groupId>
			<artifactId>scala-library</artifactId>
			<version>2.13.5</version>
		</dependency>
	</dependencies>

	<build>
		<plugins>
			<plugin>
				<artifactId>maven-compiler-plugin</artifactId>
				<version>3.8.1</version>
			</plugin>
			<plugin>
				<artifactId>maven-surefire-plugin</artifactId>
				<version>${surefire-plugin.version}</version>
			</plugin>
			<plugin>
				<groupId>net.alchim31.maven</groupId>
				<artifactId>scala-maven-plugin</artifactId>
				<version>${scala-maven-plugin.version}</version>
				<executions>
					<execution>
						<id>scala-compile-first</id>
						<phase>process-resources</phase>
						<goals>
							<goal>add-source</goal>
							<goal>compile</goal>
						</goals>
					</execution>
					<execution>
						<id>scala-test-compile</id>
						<phase>process-test-resources</phase>
						<goals>
							<goal>add-source</goal>
							<goal>testCompile</goal>
						</goals>
					</execution>
				</executions>
				<configuration>
					<scalaVersion>${scala.version}</scalaVersion>
					<args></args>
				</configuration>
			</plugin>
		</plugins>
	</build>

</project>

Output of uname -a or ver

MSYS_NT-10.0-21382 DESKTOP-88A51E5 3.1.7-340.x86_64 2020-10-23 13:08 UTC x86_64 Msys

Output of java -version

JDK 11 and JDK 16

GraalVM version (if different from Java)

GraalVM 21.2.0-dev

Quarkus version or git rev

2.0.0-FINAL

Build tool (ie. output of mvnw --version or gradlew --version)

rayga@DESKTOP-88A51E5 MSYS ~
$ mvn --version
Apache Maven 3.8.1 (05c21c65bdfed0f71a2f2ada8b84da59348c4c5d)
Maven home: C:\Program Files\apache-maven-3.8.1
Java version: 11.0.11, vendor: GraalVM Community, runtime: C:\GraalVM\graalvm-ce-java11-21.2.0-dev
Default locale: en_US, platform encoding: Cp1252
OS name: "windows 10", version: "10.0", arch: "amd64", family: "windows"
@GavinRay97 GavinRay97 added the kind/bug Something isn't working label Jul 3, 2021
@quarkus-bot quarkus-bot bot added the env/windows Impacts Windows machines label Jul 3, 2021
@GavinRay97
Copy link
Contributor Author

GavinRay97 commented Jul 3, 2021

Okay so it looks like it may be something in the implementation here that is impacting it? (I'm not really sure though, it could actually be something else entirely or in the base JUnit5 package)

Sorry for the ping, but:

@stuartwdouglas I'm happy to try to help fix whatever weirdness is going on here myself, but Quarkus is a very large project and I don't have any of the background context. Git blame says you're the fella to talk to about this file -- do you think you might be able to share any thoughts or context?

Would be super appreciated, I really want to publish my Scala 3 support extension but I don't know how useful it will be until tests work (at even some basic level). I think this is an issue with the current Scala 2 support as well so would be a sweet bonus to get that patched up =D

private DiscoveryResult discoverTestClasses(DevModeContext devModeContext) {
//maven has a lot of rules around this and is configurable
//for now this is out of scope, we are just going to do annotation based discovery
//we will need to fix this sooner rather than later though
//we also only run tests from the current module, which we can also revisit later
Indexer indexer = new Indexer();
try (Stream<Path> files = Files.walk(Paths.get(devModeContext.getApplicationRoot().getTest().get().getClassesPath()))) {
files.filter(s -> s.getFileName().toString().endsWith(".class")).forEach(s -> {
try (InputStream in = Files.newInputStream(s)) {
indexer.index(in);
} catch (IOException e) {
throw new RuntimeException(e);
}
});
} catch (IOException e) {
throw new RuntimeException(e);
}
Index index = indexer.complete();
//we now have all the classes by name
//these tests we never run
Set<String> integrationTestClasses = new HashSet<>();
for (DotName intAnno : Arrays.asList(QUARKUS_INTEGRATION_TEST, NATIVE_IMAGE_TEST)) {
for (AnnotationInstance i : index.getAnnotations(intAnno)) {
DotName name = i.target().asClass().name();
integrationTestClasses.add(name.toString());
for (ClassInfo clazz : index.getAllKnownSubclasses(name)) {
integrationTestClasses.add(clazz.name().toString());
}
}
}
Set<String> quarkusTestClasses = new HashSet<>();
for (AnnotationInstance i : index.getAnnotations(QUARKUS_TEST)) {
DotName name = i.target().asClass().name();
quarkusTestClasses.add(name.toString());
for (ClassInfo clazz : index.getAllKnownSubclasses(name)) {
if (!integrationTestClasses.contains(clazz.name().toString())) {
quarkusTestClasses.add(clazz.name().toString());
}
}
}
Set<DotName> allTestAnnotations = collectTestAnnotations(index);
Set<DotName> allTestClasses = new HashSet<>();
for (DotName annotation : allTestAnnotations) {
for (AnnotationInstance instance : index.getAnnotations(annotation)) {
if (instance.target().kind() == AnnotationTarget.Kind.METHOD) {
allTestClasses.add(instance.target().asMethod().declaringClass().name());
}
}
}
//now we have all the classes with @Test
//figure out which ones we want to actually run
Set<String> unitTestClasses = new HashSet<>();
for (DotName testClass : allTestClasses) {
String name = testClass.toString();
if (integrationTestClasses.contains(name) || quarkusTestClasses.contains(name)) {
continue;
}
ClassInfo clazz = index.getClassByName(testClass);
if (Modifier.isAbstract(clazz.flags())) {
continue;
}
unitTestClasses.add(name);
}
List<Class<?>> itClasses = new ArrayList<>();
List<Class<?>> utClasses = new ArrayList<>();
for (String i : quarkusTestClasses) {
try {
itClasses.add(Thread.currentThread().getContextClassLoader().loadClass(i));
} catch (ClassNotFoundException e) {
throw new RuntimeException(e);
}
}
itClasses.sort(Comparator.comparing(new Function<Class<?>, String>() {
@Override
public String apply(Class<?> aClass) {
ClassInfo def = index.getClassByName(DotName.createSimple(aClass.getName()));
AnnotationInstance testProfile = def.classAnnotation(TEST_PROFILE);
if (testProfile == null) {
return "$$" + aClass.getName();
}
return testProfile.value().asClass().name().toString() + "$$" + aClass.getName();
}
}));
QuarkusClassLoader cl = null;
if (!unitTestClasses.isEmpty()) {
//we need to work the unit test magic
//this is a lot more complex
//we need to transform the classes to make the tracing magic work
QuarkusClassLoader deploymentClassLoader = (QuarkusClassLoader) Thread.currentThread().getContextClassLoader();
Set<String> classesToTransform = new HashSet<>(deploymentClassLoader.getLocalClassNames());
Map<String, byte[]> transformedClasses = new HashMap<>();
for (String i : classesToTransform) {
try {
byte[] classData = IoUtil
.readBytes(deploymentClassLoader.getResourceAsStream(i.replace('.', '/') + ".class"));
ClassReader cr = new ClassReader(classData);
ClassWriter writer = new QuarkusClassWriter(cr,
ClassWriter.COMPUTE_FRAMES | ClassWriter.COMPUTE_MAXS);
cr.accept(new TestTracingProcessor.TracingClassVisitor(writer, i), 0);
transformedClasses.put(i.replace('.', '/') + ".class", writer.toByteArray());
} catch (IOException e) {
throw new RuntimeException(e);
}
}
cl = testApplication.createRuntimeClassLoader(testApplication.getAugmentClassLoader(), Collections.emptyMap(),
transformedClasses);
for (String i : unitTestClasses) {
try {
utClasses.add(cl.loadClass(i));
} catch (ClassNotFoundException exception) {
throw new RuntimeException(exception);
}
}
}
if (testType == TestType.ALL) {
//run unit style tests first
//before the quarkus tests have started
//which stops quarkus interfering with WireMock
List<Class<?>> ret = new ArrayList<>(utClasses.size() + itClasses.size());
ret.addAll(utClasses);
ret.addAll(itClasses);
return new DiscoveryResult(cl, ret);
} else if (testType == TestType.UNIT) {
return new DiscoveryResult(cl, utClasses);
} else {
return new DiscoveryResult(cl, itClasses);
}
}


Edit: I decompiled the Scala class bytecode with Recaf, expecting there to be a large difference between Java that might give a clue, but confusingly it's identical to a Java implementation except for the swapping of void with Scala's Nothing:

image

@stuartwdouglas
Copy link
Member

This is because the test method ends up looking like:

    @Test
    public ValidatableResponse testHelloEndpoint() {

And deep inside JUnit it will silently ignore methods annotated with @test that return a value other than void.

@stuartwdouglas
Copy link
Member

IMHO silently ignoring this is a JUnit 5 issue, do you want to file an issue with them?

@geoand
Copy link
Contributor

geoand commented Jul 5, 2021

This is because the test method ends up looking like:

    @Test
    public ValidatableResponse testHelloEndpoint() {

And deep inside JUnit it will silently ignore methods annotated with @test that return a value other than void.

I've been bitten by this in past as well... And it's just a bad default...

@GavinRay97
Copy link
Contributor Author

GavinRay97 commented Jul 5, 2021

Hey thank you for the reply @stuartwdouglas! 🙏
I'm happy to file this issue at JUnit instead -- could I ask for one bit of insight as to how this works though:

This is because the test method ends up looking like:

     @Test
     public ValidatableResponse testHelloEndpoint() {

I thought it might be something like this but it happens even for test cases in which I just do this:

@QuarkusTest
class SomeTest:
	@Test
    def testThing() = assert(false)

And that comes out as:

@QuarkusTest
public class SomeTest {
  @Test
  public scala.runtime.Nothing$ testThing() {
    return scala.runtime.Scala3Runtime$.MODULE$.assertFailed();
  }
}

In this scenario, it's because again, it's a public scala.runtime.Nothing$ testThing() and not a public void testThing()?

Just want to try to see if I understand it right.


EDIT: Oh wow, you're absolutely right!

I checked the decompiled version of this working test from their junit5-jupiter-starter-maven that IntelliJ auto-translated from Java -> Scala

image

image

So if : Unit is the magic trick, then... 🤔

YES!! 🥳 🎉 💯 🏆 🎊 Quarkus endpoints and tests working with Scala 3!

image

image

@GavinRay97
Copy link
Contributor Author

GavinRay97 commented Jul 5, 2021

So there is no problem here, sorry for filing an issue and the ping.

This is (if I am allowed to hold this opinion) an instance of really poor UX/DX on the side of JUnit5, as Stuart and Georgios both stated above.

I am happy to close this, finalize + submit my PR for Scala 3 support, and follow up with an issue on the JUnit repo that links back to this as evidence of why the current behavior is an awful default to have 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
env/windows Impacts Windows machines kind/bug Something isn't working triage/invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

3 participants