Do not run 'pollutedTests' task as a dependency of 'test'

[abelsromero]

pollutedTest Gradle task is set as a dependency for test. That's on it's own a bat pattern (https://melix.github.io/blog/2021/10/gradle-quickie-dependson.html) but means there's no way to run tests without it.

Imo, we should look for an alternative to run the test without resorting to this pattern. Plus, this is especially an issue when running from IDE, since the polluted test will run always when trying to run a test class or method independently.

[abelsromero]

I've found https://junit-pioneer.org/, which has an extension to inject environment variables with an annotation.

I run a test on the junit5 PR branch and both of the following options work :+1:

1. Add annotation to test directly

   @SetEnvironmentVariable(key = "GEM_PATH", value = "/some/path")
   @SetEnvironmentVariable(key = "GEM_HOME", value = "/some/other/path")
   public class WhenAnAsciidoctorClassIsInstantiatedInAnEnvironmentWithGemPath {

2. Add pioneer annotations to our custom Polluted annotation.

   @Retention(RetentionPolicy.RUNTIME)
   @Target(ElementType.TYPE)
   // @Tag("polluted")
   @SetEnvironmentVariable(key = "GEM_PATH", value = "/some/path")
   @SetEnvironmentVariable(key = "GEM_HOME", value = "/some/other/path")
   public @interface Polluted {
   }

In both cases we can remove the 'polluted' task configuration from Gradle, so WhenAnAsciidoctorClassIsInstantiatedInAnEnvironmentWithGemPath runs as any other test and the IDE issues disappear. :point_right: Less code and simpler build, I say it's a win, the only downside is we can't reuse it for v2.5.x, so as for me it will remain as it is.

Of the two options, I personally prefer the second, which adds some semantics to our discussions. We can refer to "polluted tests" and know exactly what it is. So I'd suggest proceeding with that as soon as JUnit5 is merged.

[robertpanzer]

Let's go with the second option then. I am somewhat curious how it should be possible to run a test with a different class path if this is an anti pattern? (And I've thought of adding sth like build.dependsOn(pollutedTest) and it did not feel right to me, if I run ./gradlew test I want all tests to run.)

[abelsromero]

Based on the article the anti-pattern is the use of dependsOn on its own, in our case there's no actual real dependency between test and pollutedTest, it's just convenient to get the later run when using the test task. But as an example of the bad consequence from that is that we cannot run them in parallel, when in fact we could. One thing we do at work is having multiple Test tasks (integration, acceptance, ...) and reserve test for units only, then we just invoke the necessary ones ./gradlew accepanceTest test. In CI it means having a longer command, locally is the issue where you need to be aware of that. That's why -while annoying- I'd rather keep it as it is in v2.5.x, not being a daily projec I don't trust myself remembering to run which task :sweat_smile: I admit I don't understand why pollutedTest task does not catch results though :thinking:

On the topic of a different classpath I think we'd need to define a Gradle configuration and use that for the other test task.