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

Allow @QuarkusTest and @QuarkusIntegrationTest to coexist in same test suite #23528

Closed
GregJohnStewart opened this issue Feb 8, 2022 · 34 comments · Fixed by #24064
Closed

Allow @QuarkusTest and @QuarkusIntegrationTest to coexist in same test suite #23528

GregJohnStewart opened this issue Feb 8, 2022 · 34 comments · Fixed by #24064
Labels
area/testing kind/enhancement New feature or request
Milestone

Comments

@GregJohnStewart
Copy link
Contributor

Description

I am running across some major hurtles with @QuarkusTest vs QuarkusIntegrationTest. Essentially, it makes sense to me as a developer that I would want to run both in the same test* command, and for the test codebase to comingle.

What I want to accomplish with this:

  • have the same tests for both native and non-native mode (possible in Maven, but not Gradle?)
  • simplify test writing, running, and configuration; nothing special for integration tests
  • potentially simplify Jacoco coverage? (Haven't looked into it, but I might think the extra config typically required for coverage of integration tests would be resolved with these proposed changes)

Currently though it seems that due to how the app is packaged for the different types of test, this is not currently supported and requires multiple code bases and different commands to be used to run them.

In Gradle, this seems to be the purpose of the native-test source set at the core, and in Maven an amount of pom configuration.

I suggest that we support a homogeneous codebase and command for running both @QuarkuTest and @QuarkusIntegrationTest tests. This would drastically simplify test creation and required configuration. If one wanted to have additional source-sets for Gradle runs it should still be possible, but one could also just run the same source set of test in native mode, and have the same tests available for jar integration testing.

I also realize I could be missing something and a reasonable solution might exist but I am not seeing it.

Implementation ideas

We already have the ability to segment/ order tests by profiles and test resources; if we added the ability to order based on the two test annotations, we can separate the required runtime contexts and ensure they can run in the same suite/ run. It is possible I have a naive view of how the tests execution/ordering is handled, but I believe this is possible based on a couple of assumptions:

  • we can carefully order the tests
  • we have an amount of control of the running instance of quarkus used for each test
@GregJohnStewart GregJohnStewart added the kind/enhancement New feature or request label Feb 8, 2022
@quarkus-bot
Copy link

quarkus-bot bot commented Feb 8, 2022

/cc @geoand

@geoand
Copy link
Contributor

geoand commented Feb 9, 2022

@QuarkusIntegrationTest should only be used for integration tests, i.e. tests run by Maven Failsafe.

Did you have encounter problems in such a setup?

@GregJohnStewart
Copy link
Contributor Author

I do see this, and I am talking about both Maven and Gradle.

The integration test idea makes sense to me; testing a fully packaged application from outside. What makes less sense is why this can't be part of the regular test command/suite.

For Gradle, any integration test that I create gets some form of initialization error when it is in the test suite, forcing me to relegate integration tests to the test-native source set, or using Gradle to setup a special source set.

(Example init error: Unable to locate the artifact metadata file created that must be created by Quarkus in order to run integration tests. Make sure this test is run after the 'quarkusBuild' Gradle task.)

For both Maven and Gradle, it seems odd to need two commands to run all of my tests. I would want to be able to have one command to ensure all my tests are run and passing, with the option to exclude potentially slower integration tests.

@geoand
Copy link
Contributor

geoand commented Feb 9, 2022

The idea is that the @QuarkusIntegrationTest tests must run after the application has been packaged. In Maven this is acomplished by having them run via failsafe (thus the IT suffix).
That way when one runs mvn verify (assuming failsafe has been configured to run for the integration-test or verify goals) the test are executed properly.
AFAIK, a similar case occurs when one invokes ./gradlew check.

So I fail to see what the proposal is here.

@GregJohnStewart
Copy link
Contributor Author

GregJohnStewart commented Feb 9, 2022

The proposal is that it should be easier to setup these integration tests; these tests should comingle in the test source sets with regular @QuarkusTests, and not require special configuration, and run in the same task (test/check/etc)

If that means making the build/ package task a dependency for the tests, I feel that is reasonable.

A note that making test dependent on quarkusBuild results in the following error, and the check task did not seem to do much for me.

Caused by: java.lang.ClassCastException: class io.quarkus.test.junit.RunningAppConfigResolver$1 cannot be cast to class io.smallrye.config.SmallRyeConfig (io.quarkus.test.junit.RunningAppConfigResolver$1 and io.smallrye.config.SmallRyeConfig are in unnamed module of loader 'app')

I am of course willing to be shown an example of how these are 'meant' to be done, and that I might be overcomplicating something, but from what I have seen I don't see a perfect solution. Maybe my google foo is off, but I can't seem to find any comprehensive documentation on the topic (something that specifically hits integration tests, rather than mentioning it in passing)

@geoand
Copy link
Contributor

geoand commented Feb 9, 2022

The proposal is that it should be easier to setup these integration tests;

I personally don't feel there is anything difficult about the current setup as it's based on standard build tool practices.

If that means making the build/ package task a dependency for the tests, I feel that is reasonable.

That is huge breaking change and from my perspective brings no real benefit.

I personally fail to see why the standard test vs verify Maven goal split is problematic. We are using these the way they were supposed to be used.

I'd like to hear what @famod and @edeandrea think.

@edeandrea
Copy link
Contributor

edeandrea commented Feb 9, 2022

So I'll chime in here with my $0.02. And for those who don't know me, I'm much more a Gradle fan than I am a Maven fan :)

Like @geoand mentions, integration tests must run after the application has been packaged, since those tests are run against whatever the output artifact is (jar file, container image, native binary, etc). This means that those test classes need to have a separate classloader.

I actually prefer the way Gradle does this by having a separate source set (the name native-test needs to change - see #22035 for that conversation). In all my history using Gradle over the last 8-9 years I've always created separate source sets to isolate unit from integration tests. This source set has separate dependencies/etc, and can "extend" the unit test source set if you want it to.

I think Maven makes this separation messy because the unit & integration tests are co-mingled in the same source set (src/test), but yet the classes still have the separate classloading. Maven also relies on 2 separate plugins for running the sets of tests and uses a naming convention to separate them. I personally don't like it, but it is what it is and is really unrelated to Quarkus.

You also mention have the same tests for both native and non-native mode - but is that the reality? In your unit tests you will test "layers" of your app in isolation, perhaps using mocking/stubbing/spying, whereas in integration tests you are interacting with your app "from the outside". That is why @QuarkusTest tests run with the test profile but the @QuarkusIntegrationTest tests run with the prod profile. I know that the projects generated by code.quarkus.io have the integration/native test extend the unit test. Personally I don't like that at all for the reasons I just mentioned in this paragraph.

For both Maven and Gradle, it seems odd to need two commands to run all of my tests. I would want to be able to have one command to ensure all my tests are run and passing, with the option to exclude potentially slower integration tests.

You don't need two commands to run your entire test suite. In maven, ./mvnw verify and in Gradle, ./gradlew build should run everything. In maven, there is a PR that is proposed that would move the maven failsafe config out of the native profile and into the main profile. See #21997 for details on that.

@edeandrea
Copy link
Contributor

edeandrea commented Feb 9, 2022

@geoand I'm playing with this a little bit. I'm attaching the project I'm using here
code-with-quarkus.zip

If I have a Gradle project and inside src/native-test/java/org/acme/ExampleResourceIT.java:

package org.acme;

import io.quarkus.test.junit.QuarkusIntegrationTest;

@QuarkusIntegrationTest
public class ExampleResourceIT extends ExampleResourceTest {

    // Execute the same tests but in native mode.
}

If I then run ./gradlew clean build, that test is NOT run. If that's how it should behave then I think that is wrong.

So maybe I see where @GregJohnStewart is coming from - having just integration tests that you want to run against the output of a Gradle build (not necessarily only against native images) requires some "manual labor" by the developer.

Following this guide shouldn't I be able to run these integration tests against whatever the output of the build is?

Here is my console log (I added some stuff to build.gradle so that the test output is sent to the console):

╰─ ./gradlew clean build --console=plain
> Task :clean
> Task :processResources

> Task :quarkusGenerateCode
preparing quarkus application

> Task :compileJava
> Task :classes
> Task :jar

> Task :quarkusGenerateCodeTests
preparing quarkus application

> Task :compileTestJava
> Task :processTestResources NO-SOURCE
> Task :testClasses

> Task :test

ExampleResourceTest STANDARD_ERROR
    Feb 09, 2022 11:52:15 AM org.jboss.threads.Version <clinit>
    INFO: JBoss Threads version 3.4.2.Final
    Feb 09, 2022 11:52:16 AM org.jboss.threads.Version <clinit>
    INFO: JBoss Threads version 3.4.2.Final
    Feb 09, 2022 11:52:17 AM io.quarkus.bootstrap.runner.Timing printStartupTime
    INFO: Quarkus 2.7.1.Final on JVM started in 2.262s. Listening on: http://localhost:8081
    Feb 09, 2022 11:52:17 AM io.quarkus.bootstrap.runner.Timing printStartupTime
    INFO: Profile test activated. 
    Feb 09, 2022 11:52:17 AM io.quarkus.bootstrap.runner.Timing printStartupTime
    INFO: Installed features: [cdi, resteasy-reactive, resteasy-reactive-jackson, smallrye-context-propagation, vertx]

ExampleResourceTest > testHelloEndpoint() STANDARD_ERROR
    Feb 09, 2022 11:52:18 AM org.acme.ExampleResourceTest testHelloEndpoint
    INFO: Inside org.acme.ExampleResourceTest.testHelloEndpoint()

ExampleResourceTest > testHelloEndpoint() PASSED

Gradle Test Executor 5 STANDARD_ERROR
    Feb 09, 2022 11:52:20 AM io.quarkus.bootstrap.runner.Timing printStopTime
    INFO: Quarkus stopped in 0.067s

> Task :quarkusBuild
building quarkus jar

> Task :assemble
> Task :check
> Task :build

This is what I added to build.gradle:

tasks.withType(Test) {
    // Configuration for all test tasks
    //makes the standard streams (err and out) visible at console when running tests
    testLogging {
        displayGranularity -1
        minGranularity -1
        maxGranularity -1
        showStandardStreams true
        showStackTraces true
        showExceptions true
        showCauses true
        exceptionFormat "full"
        stackTraceFilters "truncate", "groovy"
        events "passed", "failed", "skipped"
    }

    reports {
        junitXml.outputPerTestCase = true
    }
}

If I run ./gradlew testNative after doing a build, then the integration test runs against the JVM, but if I do ./gradlew clean testNative, then it attempts to build a native image first and then run the tests against that.

So @GregJohnStewart might be onto something - just maybe wasn't able to express it.

@geoand
Copy link
Contributor

geoand commented Feb 9, 2022

I haven't used Quarkus with Gradle in a while, so I'll have to check and come back

@GregJohnStewart
Copy link
Contributor Author

GregJohnStewart commented Feb 9, 2022

Yeah, same on preferring Gradle myself :)

I fully understand the purpose of integration tests, and makes sense for separate source sets, where the check (or similar) command would tie them together (and run both). Yes, the maven model seems to be muddying the field, and I have thusfar been trying to keep things out of native-test as it felt like not the place for the tests I am wanting to implement (integration tests runnable against any form of a packaged app). If the native-test source set is where integration tests want to live, then that's fine and I'm looking forward to the name change, but it seems that the tests in this source set are only ever run in the nativeTest task.

That being said, I also saw that with a build, the integration tests (in native-test) do not run. If we got it working where native-test becomes integration-test and are run during a check (or similar) task with an option for native/jvm/etc/(Maybe a set? run against multiple build modes in one go?) then that works with me.

I did end up finding the test quickstart that seems to clear up the Maven questions, though.

@edeandrea
Copy link
Contributor

edeandrea commented Feb 9, 2022

From my perspective the workflow you are mentioning (being able to run integration tests with a check or build task) is a valid one. You should be able to run the integration tests against whatever the output of the build is.

In my opinion what is there currently is broken. One workaround I did find is that if you do ./gradlew build and then follow it with ./gradlew nativeTest, then integration tests in the native-test source set are run against the produced jar. NOTE that ./gradlew build nativeTest doesn't work (because of the way gradle creates its task graphs).

I don't know the internals of the Quarkus Gradle plugin, but something doesn't seem to be right. The quarkusBuild task shouldn't be building a native image unless specifically asked to do so (by using the --native flag).

@geoand
Copy link
Contributor

geoand commented Feb 9, 2022

cc @glefloch who knows more about the Gradle plugin than anyone

@geoand
Copy link
Contributor

geoand commented Feb 10, 2022

@glefloch can you perhaps take a look at the issues Eric mentions?

Thanks!

@glefloch
Copy link
Member

Yes sure

@glefloch
Copy link
Member

glefloch commented Feb 10, 2022

Looking back at the plugin code, we indeed have a testNative task that depends on quarkusBuild. When running the testNative task we force the quarkus.package.type property to native, meaning that running ./gradlew clean testNative will first trigger a quarkusBuild with the system property quarkus.package.type=native.

When first running ./gradlew quarkusBuild and then ./gradlew nativeTest, @QuarkusIntegrationTest tests work because gradle won't run the quarkusBuild as it looks "UP-TO-DATE" (which is an issue with the task input/output I think)

I don't know how to handle this. buildNative and testNative tasks forcing the package type to native based on task name seems legit. But on the other hand, we are not able to run @QuarkusIntegrationTest without some special configuration in the build file (I wasn't aware of that).

As mention in #21997 should we introduce a new integration test sourceset in gradle ? and in the future start deprecating the native-test source set ?

@geoand
Copy link
Contributor

geoand commented Feb 10, 2022

As mention in #21997 should we introduce a new integration test sourceset in gradle ? and in the future start deprecating the native-test source set ?

I think so. @aloubyansky WDYT?

@glefloch
Copy link
Member

This would also fix the issue faced in #22982

@edeandrea
Copy link
Contributor

edeandrea commented Feb 10, 2022

Keep in mind though (also described in #22035) that it needs to be backwards compatible. On the Maven side, the execution of integration tests is done by the maven-failsafe-plugin, which means there really isn't much (if anything?) that the quarkus-maven-plugin does. Therefore backwards-compatibility isn't an issue because existing projects will just continue doing what they're doing. Its on the user to make changes to pom.xml.

In Gradle, however, the source set config/execution is managed by the Quarkus Gradle plugin. That plugin will need to support both the native-test source set as-is today as well as a new/renamed source set. Otherwise any apps already out there that still use native-test will be broken.

There needs to be a deprecation period in place for users to migrate away before its removed completely.

@glefloch Take a look at #22035 as well. I opened that when the conversation in #21997 spilled into discussing how similar capability should be handled in Gradle.

@glefloch
Copy link
Member

Right, we won't just rename the sourceset, we will have to create a new source set without the extra quarkus.package.type=native and then start deprecating the default native-test source set.

What do you think of first creating that new source set and then updating code start? Have a preference for the source set name ? it / integrationTest ?

@GregJohnStewart
Copy link
Contributor Author

My own opinion, but intTest is my pick, more succinct than integrationTest and more descriptive than just it

@edeandrea
Copy link
Contributor

edeandrea commented Feb 10, 2022

Have a preference for the source set name ? it / integrationTest ?

I think it is too short and integrationTest is too long :) Something in between? Personally I don't like the - in the name. Maybe intTest?

@aloubyansky
Copy link
Member

As mention in #21997 should we introduce a new integration test sourceset in gradle ? and in the future start deprecating the native-test source set ?

I think so. @aloubyansky WDYT?

Makes sense to me.

@glefloch
Copy link
Member

I'm ok with src/intTest/java, this is different from the current src/native-test/java but I'm not sure user will get both source set at the same time, so I think it's ok. @geoand, @aloubyansky do you agree ?

@edeandrea
Copy link
Contributor

I'm ok with src/intTest/java, this is different from the current src/native-test/java but I'm not sure user will get both source set at the same time, so I think it's ok. @geoand, @aloubyansky do you agree ?

@glefloch Maybe better to take that conversation over to #22035 since that ticket has been around for a bit and is specifically for this?

@glefloch
Copy link
Member

Sure i will sum up everything there. I should be able to come up with a PR quite soon.

@sdaclin
Copy link

sdaclin commented Feb 11, 2022

Hello there,
I wonder why the annotation io.quarkus.test.junit.QuarkusTest is declaring the jUnit tag io.quarkus.test.junit.QuarkusTest whereas the annotation io.quarkus.test.junit.QuarkusIntegrationTest isn't declaring any ?
To me it would very easy to have a dedicated tag to be able to include/exclude these in several test runs ?

Something like this for instance :

tasks.test {
    useJUnitPlatform {
        excludeTags("io.quarkus.test.junit.QuarkusIntegrationTest")
    }
}

tasks.register<Test>("intTest") {
    group = "verification"
    useJUnitPlatform {
        includeTags("io.quarkus.test.junit.QuarkusIntegrationTest")
    }
    shouldRunAfter("test")
}

tasks.check {
    dependsOn("intTest")
}

@geoand
Copy link
Contributor

geoand commented Feb 11, 2022

shouldRunAfter("test")

This is not enough. @QuarkusIntegrationTest ests need to run after the application artifact (jar, container or native-image) is built.

@sdaclin
Copy link

sdaclin commented Feb 11, 2022

@QuarkusIntegrationTest tests need to run after the application artifact

Definitely, it's more likely to be something like :

dependsOn("assemble")
shouldRunAfter("test")

@edeandrea
Copy link
Contributor

edeandrea commented Feb 11, 2022

Really needs to be mustRunAfter("quarkusBuild") and perhaps even build.finalizedBy("intTest")

@GregJohnStewart
Copy link
Contributor Author

Bump to this! How goes it?

@geoand
Copy link
Contributor

geoand commented Feb 28, 2022

I don't think anyone is currently working on it

@glefloch
Copy link
Member

I started some work before holidays, I will and push it. I was waiting for the maven PR to be merge before.

@edeandrea
Copy link
Contributor

The maven PR is about whether or not to always run the integration tests (instead of only in the native profile).

Currently it's impossible to even run @QuarkusIntegrationTests in Gradle right? That needs to be fixed.

Whether or not to always run them (i.e. build.finalizedBy(intTest) is a different problem. I think regardless we need the intTest tasks and source set.

Just my $0.02.

@edeandrea
Copy link
Contributor

Hi @FengHuangDong I'm not sure your comment has anything to do with the issue being discussed in this ticket? It might get more visibility if you start a discussion thread about it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/testing kind/enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants