TEST: Fix test task invocation

[original-brownbear]

./gradlew test was broken by 8557bba.

Fails with:

FAILURE: Build failed with an exception.

* What went wrong:
Found the 1 `test` tasks:
  :client:benchmark:test -> class org.gradle.api.tasks.testing.Test_Decorated

on master

I think simply removing the test task fixes things though (should be ok now since that commit made the test replace action hard against missing test tasks right?).

[elasticmachine]

Pinging @elastic/es-core-infra

[alpar-t]

Sorry for missing this.
I handled a similar case in RandomizedTestingPlugin.replaceTestTask, note that the line above that removes it from check doesn't work reliably either. I think you might be better off with test.enabled = false

[original-brownbear]

@atorok np, let me try that. Seems my approach doesn't work after all. It fixes test but some naming check fails now (likely because the replacement didn't actually happen now).

[alpar-t]

Isn't that the same naming test that's going on in CI ?

[original-brownbear]

@atorok ci failed with:
https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+pull-request/12497/console

16:07:37 Starting process 'Gradle Test Executor 2'. Working directory: /var/lib/jenkins/workspace/elastic+elasticsearch+pull-request/distribution/bwc/next-minor-snapshot/build/bwc/checkout-6.x/buildSrc Command: /var/lib/jenkins/.java/oracle-10+46-linux/bin/java -Dorg.gradle.native=false @/tmp/gradle-worker-classpath1600404273191648248txt -Dfile.encoding=UTF-8 -Duser.country=US -Duser.language=en -Duser.variant -ea worker.org.gradle.process.internal.worker.GradleWorkerMain 'Gradle Test Executor 2'
16:07:37 Successfully started process 'Gradle Test Executor 2'
16:07:51 
16:07:51 org.elasticsearch.gradle.precommit.NamingConventionsTaskIT > testNameCheckFailsAsItShouldWithMain FAILED
16:07:51     java.lang.AssertionError: expected:  'Classes ending with [Tests] or [IT] or extending [UnitTestCase] must be in src/test/java:' but it was not found in the output
16:07:51         at __randomizedtesting.SeedInfo.seed([932913760A8CAE23:BE1DB584958467F5]:0)
16:07:51         at org.junit.Assert.fail(Assert.java:88)
16:07:51         at org.junit.Assert.assertTrue(Assert.java:41)
16:07:51         at org.elasticsearch.gradle.precommit.NamingConventionsTaskIT.testNameCheckFailsAsItShouldWithMain(NamingConventionsTaskIT.java:65)
16:07:56 
16:07:56 org.elasticsearch.gradle.precommit.NamingConventionsTaskIT > testNameCheckFailsAsItShould FAILED
16:07:56     java.lang.AssertionError: expected:  'Found inner classes that are tests, which are excluded from the test runner:' but it was not found in the output
16:07:56         at __randomizedtesting.SeedInfo.seed([932913760A8CAE23:BD0BAE6730164B25]:0)
16:07:56         at org.junit.Assert.fail(Assert.java:88)
16:07:56         at org.junit.Assert.assertTrue(Assert.java:41)
16:07:56         at org.elasticsearch.gradle.precommit.NamingConventionsTaskIT.testNameCheckFailsAsItShould(NamingConventionsTaskIT.java:40)
16:07:56 WARNING: An illegal reflective access operation has occurred
16:07:56 WARNING: Illegal reflective access by org.codehaus.groovy.reflection.CachedClass (file:/var/lib/jenkins/.gradle/wrapper/dists/gradle-4.8.1-all/6fmj4nezasjg1b7kkmy10xgo2/gradle-4.8.1/lib/groovy-all-2.4.12.jar) to method java.lang.Object.finalize()
16:07:56 WARNING: Please consider reporting this to the maintainers of org.codehaus.groovy.reflection.CachedClass
16:07:56 WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
16:07:56 WARNING: All illegal access operations will be denied in a future release
16:07:56 
16:07:56 Gradle Test Executor 2 finished executing tests.
16:07:56 
16:07:56 
16:07:56 14 tests completed, 2 failed
16:07:56 
16:07:56 > Task :buildSrc:test FAILED
16:07:56 FAILURE: Build failed with an exception.
16:07:56 

on check when I just removed the test target. I now tried test.enabled=false (that def. fixed test locally for me, but not sure if it also fixes check)

[alpar-t]

LGTM. It will fix check as well the test task will be pulled in but skipped.

[original-brownbear]

@atorok still breaks the same way unfortunately :( If I run the broken target in isolation it's green with this change, but check overall fails.

Probably some load order issue with the new org.gradle.api.tasks.TaskContainer#getByNameLater?

yea I found gradle/gradle#5730.

I'll try to look into this a little, but you probably know a lot more than me here :)

[original-brownbear]

Same failure here for master https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+master+default-distro-bwc-tests/124/console in the bwc tests and distributing tests:
https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+master+default-distro/123/console

[original-brownbear]

@atorok now it's green since the failing tests were muted in master. Still good to merge? (I'd vote yes, it's kinda nasty that this precludes running individual tests via Gradle atm and not all tests run straight from the IDE)

[alpar-t]

Yes, it's good to go, I think that test never played a role with this change other than preventing testing it.
I agree it's not ideal. This is how Gradle sets it up, but since we have buildSrc as a separate project I'll consider running the tests there.

[original-brownbear]

@atorok ok thanks!