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

Unexpected default test task naming in JVM Test Suite plugin #25223

Closed
remal opened this issue May 30, 2023 · 2 comments
Closed

Unexpected default test task naming in JVM Test Suite plugin #25223

remal opened this issue May 30, 2023 · 2 comments
Labels
a:bug closed:not-fixed Indicates the issue was not fixed and is not planned to be in:test-suites Work related to the JvmTestSuite Plugin

Comments

@remal
Copy link
Contributor

remal commented May 30, 2023

Expected Behavior

The default test task name is test<capitalized-test-suite-name> (or <test-suite-name>Test).

Current Behavior

The default test task name equals the test suite name.

Context (optional)

JVM tasks in Gradle are usually named using SourceSet.getTaskName(). This approach provides consistent naming that is easily recognizable.

Let's say we add a source set integration. In this case, we have these tasks:

  • compileJava, compileTestJava, compileIntegrationJava
  • compileKotlin, compileTestKotlin, compileIntegrationKotlin
  • processResources, processTestResources, processIntegrationResources
  • checkstyleMain, checkstyleTest, checkstyleIntegration
  • pmdMain, pmdTest, pmdIntegration
  • etc

I'd say we have a clear pattern here.

However, jvm-test-suite plugin uses another approach. When we create integration test suite (by calling testing.suites.create('integration', JvmTestSuite)), this happens:

  • integration source set is created - this is expected
  • integration Test task is created - this is unexpected and not consistent with what we have for other JVM tasks (see earlier)

Some usage examples of jvm-test-suite plugin suggest using test suite names like integrationTest. The problem here is that there are a few articles on the Internet about creating source sets named integration or it. Also, there are plugins that use integration source set and integrationTest task approach (name.remal.integration-tests (my plugin, yes), com.coditory.integration-test, etc). On top of that, there are well-known Maven approaches of creating it/integration folders (not integrationTest).

So, I'd say it's more common in the Java world to have src/integration/java, than src/integrationTest/java. This means that the source set created by jvm-test-suite plugin should be integration. But having integration take name is not what developers expect to see.

Steps to Reproduce

Create a JVM test suite:

apply plugin: 'jvm-test-suite'
testing.suites.create('integration', JvmTestSuite)

This code creates a task named integration (of type Test).

Gradle version

8.1.1

Build scan URL (optional)

No response

Your Environment (optional)

No response

@eskatos
Copy link
Member

eskatos commented Jun 2, 2023

Thank you for your interest in Gradle!

This issue needs a decision from the team responsible for that area. They have been informed. Response time may vary.


The documentation demonstrate the creation of a test suite named integrationTest, which in turn creates an integrationTest source set in src/integrationTest and registers a Test task named integrationTest.
https://docs.gradle.org/current/userguide/jvm_test_suite_plugin.html#sec:declare_an_additional_test_suite

If you register a test suite named integration you'll get an integration task. I can understand how this can be seen as an inconsistency wrt the conventions used to name other tasks.

@eskatos eskatos added in:test-suites Work related to the JvmTestSuite Plugin 👋 team-triage Issues that need to be triaged by a specific team and removed to-triage labels Jun 2, 2023
@tresat
Copy link
Member

tresat commented Jun 2, 2023

Thank you for your feedback, unfortunately we have decided not to make this change. There are 2 main reasons why.

First, we think it's important to be able to easily identify a source set containing test code vs. one containing production code, outside of the context of an IDE. The test suffix allows for doing that without mentally parsing the build. In your example integration alone could be a valid name for production code that, well, integrates something. This would not be unusual behavior. Using test as a suffix to identify different types of test source is also a common suggestion and usage pattern by various plugins, so it isn't such a clear choice which is more conventional.

Second, and more importantly, we think of the name integrationTest for the task that runs an integrationTest suite as a mere implementation detail. You can configure the test task within a suite using:

targets { 
    all {
        testTask.configure {
            shouldRunAfter(test)
        }
    }
}

You can wire the test task to other tasks using:

tasks.named("check") { 
    dependsOn(testing.suites.named("integrationTest"))
}

In neither of those cases are you referencing the task by name, you're only working with the suite.

It's true that when using the CLI you still must name the task to run it (unless it is run indirectly as a dependency of another lifecycle task, like check as shown above). We are considering various approaches to avoid this, but it's a compromise we're willing to live with for now.

@tresat tresat closed this as not planned Won't fix, can't repro, duplicate, stale Jun 2, 2023
@tresat tresat added the closed:not-fixed Indicates the issue was not fixed and is not planned to be label Jun 2, 2023
@ov7a ov7a removed the 👋 team-triage Issues that need to be triaged by a specific team label Jun 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:bug closed:not-fixed Indicates the issue was not fixed and is not planned to be in:test-suites Work related to the JvmTestSuite Plugin
Projects
None yet
Development

No branches or pull requests

4 participants