quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.64k stars 2.65k forks source link

By default run the same tests in continuous testing as in `mvn test` #22902

Open markus-seidl opened 2 years ago

markus-seidl commented 2 years ago

Edit by @famod: Rescoped, title changed from originally: "mvn test only includes tests that end with "Test""

Describe the bug

When executing "./mvnw clean test" or "./mvnw clean verify" Quarkus includes only tests that have the suffix "Test". While "./mvnw clean compile quarkus:dev" includes also tests that have other suffixes.

Expected behavior

Include all tests that are specified and not explicitly excluded. Similar behaviour of mvn test / quarkus:dev (If possible)

Actual behavior

Tests that do not end with "Test" are not executed.

How to Reproduce?

  1. Download a new RestEasy (extension quarkus-resteasy included)
  2. Rename the default test GreetingResourceTest to GreetingResourceTestQQ
  3. Test will no longer be executed at all

Output of uname -a or ver

No response

Output of java -version

openjdk version "17.0.1" 2021-10-19 OpenJDK Runtime Environment Homebrew (build 17.0.1+1) OpenJDK 64-Bit Server VM Homebrew (build 17.0.1+1, mixed mode, sharing)

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.6

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

Apache Maven 3.8.4 (9b656c72d54e5bacbed989b64718c159fe39b537)

Additional information

No response

famod commented 2 years ago

maven-surefire-plugin has a default include filter:

Maybe continuous testing should execute fewer tests by default (matching surefire defaults), but then again someone could reconfigure surefire and things would deviate again (unless surefire config is parsed by Quarkus). There are also config properties to tune what's included in continuous testing, so you should probably do the alignment yourself.

@geoand WDYT about this?

geoand commented 2 years ago

Right, I don't think we align automatically.

famod commented 2 years ago

Question is: Do we want to align? If yes we should probably take Gradle into account as well. No clue what includes are applied there (/cc @glefloch). In any case, I consider this a continuous testing enhancement, not a bug.

geoand commented 2 years ago

Ideally yeah, but my guess is that it's not easy to make it work all the time

quarkus-bot[bot] commented 2 years ago

/cc @stuartwdouglas

famod commented 2 years ago

I took the liberty to rescope this issue. Let's see what @stuartwdouglas thinks about this when he's back. /cc also @aloubyansky, just in case.

@markus-seidl thanks for bringing this up!

famod commented 2 years ago

Btw, @markus-seidl the cont testing propery I was referring to is quarkus.test.include-pattern: https://quarkus.io/guides/continuous-testing#quarkus-test-dev-testing-test-config_quarkus.test.include-pattern Together with the surefire property you should be able to align.

aloubyansky commented 2 years ago

I think this makes sense.

markus-seidl commented 2 years ago

(redacted as it contained errors)

glefloch commented 2 years ago

Gradle runs all tests found in src/test/java, it does not depend on the class name.

stuartwdouglas commented 2 years ago

If we were going to do this it would need to be passed in from Maven I think, as we don't want to apply maven rules to Gradle projects.

Maybe it is fine to just have this documented somwhere, as you can always manually specify the include pattern if it happens to be causing you issues.

aloubyansky commented 2 years ago

I think we can document it for now. OTOH, we do have ApplicationModel which is a kind of bridge to the underlying build tool and is used to collect build system-specific data which is then used to bootstrap the app in dev, test and prod modes. So it could be supported, imo.