Remove deprecation warnings to prepare for Gradle 5 (sourceSets.main.output.classesDirs) #30389

alpar-t · 2018-05-04T13:19:50Z

Gradle replaced project.sourceSets.main.output.classesDir of type
File with project.sourceSets.main.output.classesDirs of type
FileCollection
(see SourceSetOutput)
Build output is now stored on a per language folder.

There are a few places where we use that, here's these and how it's
fixed:

Randomized Test execution
- look in all test folders ( pass the multi dir configuration to the
  ant runner )
- DRY the task configuration by introducing basedOn for
  RandomizedTestingTask DSL
Extend the naming convention test to support passing in multiple
directories
Fix the standalon test plugin, the dires were not passed trough,
checked with a debuger and the statement had no affect due to a
missing =.

Closes #30354

elasticmachine · 2018-05-04T13:20:27Z

Pinging @elastic/es-core-infra

Gradle replaced `project.sourceSets.main.output.classesDir` of type `File` with `project.sourceSets.main.output.classesDirs` of type `FileCollection` (see [SourceSetOutput](https://github.com/gradle/gradle/blob/master/subprojects/plugins/src/main/java/org/gradle/api/tasks/SourceSetOutput.java)) Build output is now stored on a per language folder. There are a few places where we use that, here's these and how it's fixed: - Randomized Test execution - look in all test folders ( pass the multi dir configuration to the ant runner ) - DRY the task configuration by introducing `basedOn` for `RandomizedTestingTask` DSL - Extend the naming convention test to support passing in multiple directories - Fix the standalon test plugin, the dires were not passed trough, checked with a debuger and the statement had no affect due to a missing `=`. Closes #30354

dadoonet · 2018-05-04T13:30:49Z

Related to #27250.

rjernst

Thanks for starting on this! I've left some comments. More generally, I see a lot of unrelated changes here. Things like reformatting or restructuring should generally go in separate PRs to ease reviewing.

rjernst · 2018-05-04T22:57:11Z

buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingPlugin.groovy

@@ -44,24 +44,24 @@ class RandomizedTestingPlugin implements Plugin<Project> {
        }
    }

-    static void replaceTestTask(TaskContainer tasks) {
+    static void replaceTestTask(Project project) {
+        def tasks = project.tasks


We generally try to avoid def. Please use concrete types.

rjernst · 2018-05-04T22:58:34Z

buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingTask.groovy

@@ -211,7 +224,7 @@ class RandomizedTestingTask extends DefaultTask {

        DefaultLogger listener = null
        ByteArrayOutputStream antLoggingBuffer = null
-        if (logger.isInfoEnabled() == false) {
+        if (!logger.isInfoEnabled()) {


We use == false for negation throughout the codebase because it is easier to visually notice.

rjernst · 2018-05-04T23:01:06Z

buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.groovy

+
+        def dirsToUse = (checkForTestsInMain ?
+            project.sourceSets.main.output.classesDirs :
+            project.sourceSets.test.output.classesDirs)


I don't think we can check these here, this would still be during configuration? Certainly not the existence of them in the filter below.

This works because the FileCollection is a live collection, this the checks are actually only executed lather.
I'm changing this anyhow, as I discovered that we should not pick up non Java tests anyhow, because those don't adhere to the naming conventions.

rjernst · 2018-05-04T23:05:08Z

buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingTask.groovy

@@ -95,6 +97,17 @@ class RandomizedTestingTask extends DefaultTask {
        listenersConfig.listeners.add(new TestReportLogger(logger: logger, config: testLoggingConfig))
    }

+    void basedOn(RandomizedTestingTask other) {
+        configure(BuildPlugin.commonTestConfig(project))


BuildPlugin should not be referred to here. The intention has always been to move the randomized testing gradle code (notice this is in a different java package than the rest of the buildSrc code) back upstream with randomized testing itself. See https://github.com/randomizedtesting/gradle-randomized-testing-plugin where I made some snapshots when first working on migrating the build to gradle, but was not able to cleanup and document it enough to release on its own.

I moved it back to the Gradle script.

rjernst · 2018-05-04T23:14:17Z

buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingTask.groovy

+    void basedOn(RandomizedTestingTask other) {
+        configure(BuildPlugin.commonTestConfig(project))
+        classpath = other.classpath;
+        testClassesDirs = other.testClassesDirs;


I think all the users of this could just be converted to use this line? I don't think we need to or should overly abstract this "shared code". For example, it would be better to make internalClusterTest task added by an additional gradle plugin to reduce duplication there.

I did change just the line everywhere during development, and decided to take the extra step to refactor it as I felt it's too detailed and does not convey intent well enough, every time I encountered it I had to carefully check to make sure none of the configuration is missing or slightly different. I do agree that a plugin as you suggest would be even better, but this is a step that will make that a bit easier to implement.

- Name checker was ran for Groovy tests that don't adhere to the same convections causing the check to fail - implement PR feedback

rjernst

I left a few more comments

rjernst · 2018-05-09T08:48:39Z

buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingTask.groovy

@@ -211,7 +223,7 @@ class RandomizedTestingTask extends DefaultTask {

        DefaultLogger listener = null
        ByteArrayOutputStream antLoggingBuffer = null
-        if (logger.isInfoEnabled() == false) {
+        if (false == logger.isInfoEnabled()) {


I still see lots of arbitrary syntactical changes like this. Every extra line of diff makes something that must be looked at. Without these minor changes, I think this PR would only be a dozen lines or so and would be very quick to review.

I don't have a problem omitting them if that's the consensus, my take on it is that the cost of making and reviewing these is worthwhile if they improve the readability of the code, as it eventually pays off.

rjernst · 2018-05-09T08:51:01Z

buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingTask.groovy

@@ -95,6 +97,16 @@ class RandomizedTestingTask extends DefaultTask {
        listenersConfig.listeners.add(new TestReportLogger(logger: logger, config: testLoggingConfig))
    }

+    void basedOn(RandomizedTestingTask other) {


I really would like to not have this. It makes many assumptions (eg that that check should depend on this). Can we please keep this change only to updating testClassesDirs and look at ways to deduplicate internalClusterTest in a separate issue?

Removed it. I have a better understanding of why this was not a good idea now, and why this would be better handled in it's own PR. There are multiple ways to manage tests, deal with unit and integrations tests etc ( there's an additionalTests extension that is similar to what basedOn did ). It might also be worth setting up a different source path for integration tests then we could auto wire that to check and offer a clear DSL for defining new tests that are possibly a subset of the existing unit or integration with additional system properties.

- Apply workaround for: GradleUp/shadow#336 - bump plugin to 2.0.4 Changes between 2.0.2 and 2.0.4 of the plugin: ``` 477db40 12 days ago john.engelman@target.com Release 2.0.4 3e3da37 3 weeks ago john.engelman@target.com Remove internal Gradle API and annotation internal getters on shadow jar. 31e2380 3 weeks ago john.engelman@target.com Close input streams. Closes elastic#364 f712cc8 3 weeks ago john.engelman@target.com Upgrade ASM to 6.1.1 to address perf issues. Closes elastic#374 2f94b2b 3 weeks ago john.engelman@target.com next version 23bbf3d 7 weeks ago john.r.engelman@gmail.com Add some gradle versions. Update changelog for 2.0.3 7435c74 7 weeks ago john.r.engelman@gmail.com Merge pull request elastic#367 from ttsiebzehntt/366-java10 325c002 7 weeks ago info@martinsadowski.de Update ASM to 6.1 94550e5 3 months ago john.r.engelman@gmail.com Merge pull request elastic#356 from sgnewson/update-file-to-files 66b691e 4 months ago john.r.engelman@gmail.com Merge pull request elastic#358 from 3flex/patch-1 14761b1 4 months ago 3flex@users.noreply.github.com fix markdown for User Guide URL in issue template a3f6984 4 months ago newson@synopsys.com update inputs.file to inputs.files, to remove warning ``` closes elastic#30389

* Solve Gradle deprecation warnings around shadowJar - Apply workaround for: GradleUp/shadow#336 - bump plugin to 2.0.4 Changes between 2.0.2 and 2.0.4 of the plugin: ``` 477db40 12 days ago john.engelman@target.com Release 2.0.4 3e3da37 3 weeks ago john.engelman@target.com Remove internal Gradle API and annotation internal getters on shadow jar. 31e2380 3 weeks ago john.engelman@target.com Close input streams. Closes #364 f712cc8 3 weeks ago john.engelman@target.com Upgrade ASM to 6.1.1 to address perf issues. Closes #374 2f94b2b 3 weeks ago john.engelman@target.com next version 23bbf3d 7 weeks ago john.r.engelman@gmail.com Add some gradle versions. Update changelog for 2.0.3 7435c74 7 weeks ago john.r.engelman@gmail.com Merge pull request #367 from ttsiebzehntt/366-java10 325c002 7 weeks ago info@martinsadowski.de Update ASM to 6.1 94550e5 3 months ago john.r.engelman@gmail.com Merge pull request #356 from sgnewson/update-file-to-files 66b691e 4 months ago john.r.engelman@gmail.com Merge pull request #358 from 3flex/patch-1 14761b1 4 months ago 3flex@users.noreply.github.com fix markdown for User Guide URL in issue template a3f6984 4 months ago newson@synopsys.com update inputs.file to inputs.files, to remove warning ``` closes #30389 * Improove comment as suggested

alpar-t · 2018-05-11T07:20:47Z

That was an incorrect issue reference, reopening.
@rjernst thanks for noticing.

This worked because the list is passed to `project.files` that does the right thing.

This reverts commit 9bd9389.

Previus revert accidentally reverted too much

alpar-t · 2018-05-16T05:59:54Z

build this please

alpar-t · 2018-05-16T06:00:31Z

test this please

alpar-t · 2018-05-16T10:33:49Z

test this please

alpar-t · 2018-05-16T13:35:22Z

test this please

rjernst · 2018-05-18T16:27:54Z

buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.groovy

@@ -66,7 +65,7 @@ public class NamingConventionsTask extends LoggedExec {
    @Input
    boolean checkForTestsInMain = false;

-    public NamingConventionsTask() {
+    NamingConventionsTask() {


Please add back the public. As I mentioned before, a goal here is to keep these files as close as possible to java so buildSrc can be converted completely to java (hopefully someday soon).

@rjernst ++ on converting to Java, or away from Groovy anyhow. Was Kotlin considered ? Gradle itself seems to be picking it up as well, we could switch to Kotlin-DSL as well and have type save build files all the way trough. It would get us the best of both words at the expense of a learning curve for Kotlin.

Kotlin was not around when we switched to gradle. We have tried sticking to java for multiple reasons:

Expertise on Elasticsearch is usually in java

I have seen numerous reports of double digit performance gains in configuration time from switching gradle plugins to java

Remember we are only talking about buildSrc here. The build.gradle files are still groovy. I am not averse to them being Kotlin, but I would want to see a significant advantage to that work, as we have now spent over 2 years with developers learning how to write groovy in gradle DSL.

rjernst · 2018-05-18T16:30:22Z

buildSrc/src/main/java/org/elasticsearch/test/NamingConventionsCheck.java

-            check.checkMain(rootPath);
-        } else {
-            check.checkTests(rootPath, skipIntegTestsInDisguise);
+        for (String rootDir : rootPathList.split(Pattern.quote(File.pathSeparator))) {


This seems inconsistent with the gradle code. There, a single dir is currently passed in. But here it looks like the path of that dir is split into its elements but then each read as a dir...

It's split with File.pathSeparator not File.separator, so multiple dirs can be passed in like a classpath.
The initial intent was to pass in FileCollection.getAsPath on sourcesDirs, and have the name check run on all directories. I backed out of that, because that sent in some groovy tests that did not adhere to the name checks, and it did not seem to make sense to make them do. We could revert this change altogether, but I taught it might be useful some day, i.e. we could collect and combine all FileCollections from all the projects output dirs to call the name checker only once and save some time on JVM startups.

I don't think we should do that. Each project should be checkable on its own. Combining all the sources into one task means to run precommit on a project requires running this one large task that looks at all test classes.

ok, I'll revert changes to this file.

rjernst · 2018-05-18T16:33:27Z

buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.groovy

+        // see https://docs.gradle.org/current/dsl/org.gradle.api.tasks.SourceSetOutput.html the deprecated property
+        // classesDir is the first in the FileCollection. There seems to be no other way to refer to the Java output dir
+        // We can't run the check against all classes as Groovy tests don't adhere to naming conventions.
+        return classesDirs[0]


Why don't we change the java code to work on multiple directories? Then you can just pass in all the classesDirs as multiple arguments.

We don't want to pass in the groovy test dirs, they would not pass the checks.

Where do we have have groovy tests? How do we know the ordering would always result in the java classes dir coming first?

we have groovy ./buildSrc/src/test/groovy/. It's an odd that the Gradle API does not offer a more explicit way to get the java specific dirs, and choose to document that it will always be first here:
https://docs.gradle.org/current/dsl/org.gradle.api.tasks.SourceSetOutput.html
It mentions the following for the deprectaed File classesDir property:

Default with java plugin: First directory from classesDirs

While that deprecated property may document that fact, there would be nothing preventing gradle from changing the order once that property is removed. There are only 2 groovy tests. I think we can update their names to follow the naming convention?

@rjernst I updated the task to pass along all dirs, but also added a new input to be able to bypass that and be specific of the directory. Adding the Groovy tests is trickier because of the inheritance check but we'll be ok, since the build tools that have a hard coded path now are self tests, so if the test would change the test would fail.

Only run on Java for build-tools, this is safe because it's a self test. It needs more work before we could pass in the Groovy classes as well as these inherit from `GroovyTestCase`

The self complicates the task setup and disable real checks on build-tools. With this change there are no more self tests, and the build-tools tests adhere to the conventions. The self test will be replaced by gradle test kit, thus the addition of the Gradle plugin builder plugin.

alpar-t · 2018-05-30T17:06:08Z

I wasn't really happy about this solution as I realized that it makes it hard to add java tests to the buildSrc which would be a big no-no. I decided to get rid of the self test in favor of a proper integration test with gradles testkit. As part of the change I also converted some files to Java using automated IDE tools to better prove the point of java test - java class and updated the Groovy tests to

alpar-t · 2018-05-31T09:51:08Z

test this please

alpar-t · 2018-05-31T13:33:53Z

test this please

rjernst

This is looking closer, but I have a few more comments.

rjernst · 2018-06-21T17:30:12Z

buildSrc/src/main/groovy/org/elasticsearch/gradle/VersionProperties.java

+/**
+ * Accessor for shared dependency versions used by elasticsearch, namely the elasticsearch and lucene versions.
+ */
+public class VersionProperties {


It appears this is being moved to a java file, but it is still under src/main/groovy?

Yes. That is because the Groovy compiler knows how to deal with this and delegate these files to the Java compiler. The reason these can't move to src/main/java (yet) us that they make use of Groovy classes that are not on the cp when pure java is compiled. Moving those Groovy classes to Java creates a ripple effect, so my thinking is that we can have java files in the groovy source set until we have critical mass, eventually removing the groovy source set. It's easier and less risky than converting to Java at once.

This sounds like a great plan!

rjernst · 2018-06-21T17:30:37Z

buildSrc/src/main/groovy/org/elasticsearch/gradle/LoggedExec.java

+/**
+ * A wrapper around gradle's Exec task to capture output and log on error.
+ */
+public class LoggedExec extends Exec {


This appears to be moved to a java file, but still exists under src/main/groovy?

rjernst · 2018-06-21T17:33:31Z

buildSrc/src/main/java/org/elasticsearch/test/NamingConventionsCheck.java

        }
+        System.exit(exitCode);


I don't think we should call System.exit. Instead, throw a GradleException with a message indicating there were violations.

This uses an exit code since we don't call into this code directly, but run it like a cli utility, it used System.exit before this change as well.

rjernst · 2018-06-21T17:35:24Z

buildSrc/src/test/groovy/org/elasticsearch/gradle/VersionCollectionTests.groovy

@@ -16,6 +20,7 @@ class VersionCollectionTest extends GroovyTestCase {
   * branched from Major-1.x At the time of this writing 6.2 is unreleased and 6.3 is the 6.x branch. This test simulates the behavior
   * from 7.0 perspective, or master at the time of this writing.
   */
+  @Test


I think this will be confusing to start having @test in gradle tests, when we do not have them in the rest of the tests (and it is unclear how this would work when we run this as a normal elasticsearch.build as the build-tools project that uses randomized runner). We should be able to annotate the test with a junit3 compatible selector like randomized runner does?

We could inherit from TestCase but we loose @Rule and probably @Before, @After etc.
The rest of the code supports both @Test and methods starting with test, but this is implemented in @RunWith(RandomizedRunner.class), @TestMethodProviders is not a junit4 annotation, the junit3 compatible selector is part of lucene test, so while it's possible to bring that in as a dependency it seems a bit odd. The other possibility is to duplicate that one filter class.

I would duplicate the class if necessary. Having @test in some parts of our code would be very confusing.

rjernst · 2018-06-21T17:36:39Z

buildSrc/src/test/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTaskTest.groovy

+import org.junit.Test
+import org.junit.rules.ExpectedException
+
+import static org.junit.Assert.*


Please do not use wildcard imports. We disallowed them in the rest of the codebase.

I fixed my IDE setting some time ago, this must pre-date that. Fixed now.

The check requires this

rjernst

LGTM, a few last comments.

rjernst · 2018-06-27T20:38:59Z

buildSrc/src/test/groovy/org/elasticsearch/gradle/VersionCollectionTests.groovy

+import org.elasticsearch.gradle.test.GradleUnitTestCase
+import org.junit.Test
+
+import static org.junit.Assert.*


Still have a wildcard import

rjernst · 2018-06-27T20:39:14Z

buildSrc/src/test/groovy/org/elasticsearch/gradle/doc/RestTestFromSnippetsTaskTests.groovy

 import static org.elasticsearch.gradle.doc.RestTestsFromSnippetsTask.replaceBlockQuote
+import static org.elasticsearch.gradle.doc.RestTestsFromSnippetsTask.shouldAddShardFailureCheck
+import static org.junit.Assert.*


Still have a wildcard import

rjernst · 2018-06-27T20:40:28Z

buildSrc/src/test/java/org/elasticsearch/gradle/test/BaseTestCase.java

+    JUnit4MethodProvider.class,
+    JUnit3MethodProvider.class
+})
+public abstract class BaseTestCase {


You can make this extend from Assert to not have to import the common methods used like assertEquals

I'll fix these, thanks, I finally figured out where these are coming from, it's in the template for test classes.

rjernst · 2018-06-27T20:41:21Z

buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleUnitTestCase.java

+    JUnit4MethodProvider.class,
+    JUnit3MethodProvider.class
+})
+public abstract class GradleUnitTestCase extends BaseTestCase  {


Why have this and BaseTestCase? I think just having one of these would be fine?

An integration test is not a unit test, so I didn't initially want them to inherit from each-other, but would have to extend the name checker to be able to deal with it.

…output.classesDirs) (#30389) * Remove deprecation warnings to prepare for Gradle 5 Gradle replaced `project.sourceSets.main.output.classesDir` of type `File` with `project.sourceSets.main.output.classesDirs` of type `FileCollection` (see [SourceSetOutput](https://github.com/gradle/gradle/blob/master/subprojects/plugins/src/main/java/org/gradle/api/tasks/SourceSetOutput.java)) Build output is now stored on a per language folder. There are a few places where we use that, here's these and how it's fixed: - Randomized Test execution - look in all test folders ( pass the multi dir configuration to the ant runner ) - DRY the task configuration by introducing `basedOn` for `RandomizedTestingTask` DSL - Extend the naming convention test to support passing in multiple directories - Fix the standalon test plugin, the dires were not passed trough, checked with a debuger and the statement had no affect due to a missing `=`. Closes #30354 * Only check Java tests, PR feedback - Name checker was ran for Groovy tests that don't adhere to the same convections causing the check to fail - implement PR feedback * Replace `add` with `addAll` This worked because the list is passed to `project.files` that does the right thing. * Revert "Only check Java tests, PR feedback" This reverts commit 9bd9389. * Remove `basedOn` helper * Bring some changes back Previus revert accidentally reverted too much * Fix negation * add back public * revert name check changes * Revert "revert name check changes" This reverts commit a2800c0. * Pass all dirs to name check Only run on Java for build-tools, this is safe because it's a self test. It needs more work before we could pass in the Groovy classes as well as these inherit from `GroovyTestCase` * remove self tests from name check The self complicates the task setup and disable real checks on build-tools. With this change there are no more self tests, and the build-tools tests adhere to the conventions. The self test will be replaced by gradle test kit, thus the addition of the Gradle plugin builder plugin. * First test to run a Gradle build * Add tests that replace the name check self test * Clean up integ test base class * Always run tests * Align with test naming conventions * Make integ. test case inherit from unit test case The check requires this * Remove `import static org.junit.Assert.*`

* master: Docs: Remove duplicate test setup Print output when the name checker IT fails (#31660) Fix syntax errors in get-snapshots docs (#31656) Docs: Fix description of percentile ranks example example (#31652) Add MultiSearchTemplate support to High Level Rest client (#30836) Add test for low-level client round-robin behaviour (#31616) SQL: Refactor package names of sql-proto and sql-shared-proto projects (#31622) Remove deprecation warnings to prepare for Gradle 5 (sourceSets.main.output.classesDirs) (#30389) Correct integTest enable logic (#31646) Fix missing get-snapshots docs reference #31645 Do not check for Azure container existence (#31617) Merge AwsS3Service and InternalAwsS3Service in a S3Service class (#31580) Upgrade gradle wrapper to 4.8 (#31525) Only set vm.max_map_count if greater than default (#31512) Add Get Snapshots High Level REST API (#31537) QA: Merge query-builder-bwc to restart test (#30979) Update reindex.asciidoc (#31626) Docs: Skip xpack snippet tests if no xpack (#31619) mute CreateSnapshotRequestTests HLRest: Fix test for explain API [TEST] Fix RemoteClusterConnectionTests Add Create Snapshot to High-Level Rest Client (#31215) Remove legacy MetaDataStateFormat (#31603) Add explain API to high-level REST client (#31387) Preserve thread context when connecting to remote cluster (#31574) Unify headers for full text queries Remove redundant 'minimum_should_match' JDBC driver prepared statement set* methods (#31494) [TEST] call yaml client close method from test suite (#31591)

Relates #30389

* 6.x: Docs: Remove duplicate test setup Docs: Fix description of percentile ranks example example (#31652) Remove deprecation warnings to prepare for Gradle 5 (sourceSets.main.output.classesDirs) (#30389) Do not check for Azure container existence (#31617) Merge AwsS3Service and InternalAwsS3Service in a S3Service class (#31580) Remove item listed in 6.3 notes that's not in 6.3 (#31623) remove unused import Upgrade gradle wrapper to 4.8 (#31525) Only set vm.max_map_count if greater than default (#31512) QA: Merge query-builder-bwc to restart test (#30979) Docs: Skip xpack snippet tests if no xpack (#31619) [TEST] Fix RemoteClusterConnectionTests Remove legacy MetaDataStateFormat (#31603) [TEST] call yaml client close method from test suite (#31591) [TEST] Close additional clients created while running yaml tests (#31575) Node selector per client rather than per request (#31471) Preserve thread context when connecting to remote cluster (#31574) Remove redundant 'minimum_should_match' Unify headers for full text queries JDBC driver prepared statement set* methods (#31494) add logging breaking changes from 5.3 to 6.0 (#31583) Fix a formatting issue in the docvalue_fields documentation. (#31563) Improve robustness of geo shape parser for malformed shapes QA: Create xpack yaml features (#31403)

Relates #30389

alpar-t added the :Delivery/Build Build or test infrastructure label May 4, 2018

alpar-t added the >non-issue label May 4, 2018

alpar-t force-pushed the fix/30354 branch from e8ed26d to 8b1affc Compare May 4, 2018 13:22

rjernst requested changes May 4, 2018

View reviewed changes

alpar-t added 2 commits May 7, 2018 16:51

Merge branch 'master' into fix/30354

684234b

Only check Java tests, PR feedback

9bd9389

- Name checker was ran for Groovy tests that don't adhere to the same convections causing the check to fail - implement PR feedback

alpar-t changed the title ~~Remove deprecation warnings to prepare for Gradle 5~~ Remove deprecation warnings to prepare for Gradle 5 (sourceSets.main.output.classesDirs) May 7, 2018

Merge branch 'master' into fix/30354

a7e641c

rjernst reviewed May 9, 2018

View reviewed changes

alpar-t mentioned this pull request May 9, 2018

Solve Gradle deprecation warnings around shadowJar #30483

Merged

alpar-t closed this in #30483 May 10, 2018

alpar-t reopened this May 11, 2018

alpar-t added 5 commits May 15, 2018 12:37

Replace add with addAll

2149a16

This worked because the list is passed to `project.files` that does the right thing.

Revert "Only check Java tests, PR feedback"

b8bf651

This reverts commit 9bd9389.

Remove basedOn helper

f9bf8fa

Bring some changes back

af3ac3e

Previus revert accidentally reverted too much

Fix negation

f514cc8

Merge commit '9434f25' into fix/30354

c0bf1af

rjernst requested changes May 18, 2018

View reviewed changes

add back public

32a6438

alpar-t added 5 commits May 28, 2018 11:41

Pass all dirs to name check

9f56f93

Only run on Java for build-tools, this is safe because it's a self test. It needs more work before we could pass in the Groovy classes as well as these inherit from `GroovyTestCase`

Merge commit 'e888467d0a7af4115789d08e4cd96bb98375968e' into fix/30354

8265b0b

First test to run a Gradle build

a1f93cf

Add tests that replace the name check self test

4b6cb42

Merge commit '8e4ab82' into fix/30354

ced99bf

rjernst reviewed Jun 21, 2018

View reviewed changes

alpar-t added 5 commits June 25, 2018 15:51

Merge commit '8b698f0' into fix/30354

a3f1cfc

Clean up integ test base class

b7f8a1a

Always run tests

c179844

Align with test naming conventions

1712976

Make integ. test case inherit from unit test case

9578cb8

The check requires this

rjernst approved these changes Jun 27, 2018

View reviewed changes

alpar-t added 2 commits June 28, 2018 11:57

Remove import static org.junit.Assert.*

72fb7a9

Merge branch 'master' into fix/30354

572dbe8

alpar-t merged commit 0afec8f into master Jun 28, 2018

alpar-t deleted the fix/30354 branch June 28, 2018 12:14

alpar-t added v7.0.0 v6.4.0 labels Jun 28, 2018

dnhatn added a commit that referenced this pull request Jun 28, 2018

Replaces testClassesDir with testClassesDirs in ccr build

1185ddb

Relates #30389

dnhatn added a commit that referenced this pull request Jun 28, 2018

Replaces testClassesDir with testClassesDirs in ccr build

947aa5a

Relates #30389

colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020

Remove deprecation warnings to prepare for Gradle 5 (sourceSets.main.output.classesDirs) #30389

Remove deprecation warnings to prepare for Gradle 5 (sourceSets.main.output.classesDirs) #30389

Conversation

alpar-t commented May 4, 2018 • edited Loading

elasticmachine commented May 4, 2018

dadoonet commented May 4, 2018

rjernst left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rjernst left a comment

Choose a reason for hiding this comment

rjernst May 9, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rjernst May 9, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alpar-t commented May 11, 2018

alpar-t commented May 16, 2018

alpar-t commented May 16, 2018

alpar-t commented May 16, 2018

alpar-t commented May 16, 2018

Choose a reason for hiding this comment

alpar-t May 21, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alpar-t commented May 30, 2018

alpar-t commented May 31, 2018

alpar-t commented May 31, 2018

rjernst left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alpar-t Jun 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rjernst left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alpar-t commented May 4, 2018 •

edited

Loading

rjernst May 9, 2018 •

edited

Loading

rjernst May 9, 2018 •

edited

Loading

alpar-t May 21, 2018 •

edited

Loading

alpar-t Jun 25, 2018 •

edited

Loading