Skip to content

Converting randomized testing to create a separate unitTest task instead of replacing the builtin test task #36311

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Dec 19, 2018
Merged

Converting randomized testing to create a separate unitTest task instead of replacing the builtin test task #36311

merged 7 commits into from
Dec 19, 2018

Conversation

@alpar-t
Copy link
Contributor

@alpar-t alpar-t commented Dec 6, 2018

Gradle 5.0 deprecates removing dependencies from tasks and replacing tasks with a different implementation.
This means that replacing the test task will be impossible for Gradle 6.0.

This PR introduced a unitTest task instead. The original test is now disabled and depended on unitTest. This means that running ./gradlew test will work as before.
With this change we will be able to continue upgrading Gradle regardless of #31496.
I also think that the test tasks name is too broad anyhow and probably only kept for historical reasons and because different projects split the tests differently.
With this PR test will be much more like check just a way to group other tasks together.

Closes #33343

@elasticmachine
Copy link
Collaborator

elasticmachine commented Dec 6, 2018

Pinging @elastic/es-core-infra

@jasontedor
Copy link
Member

jasontedor commented Dec 11, 2018

Can you address the conflicts in x-pack/plugin/ccr/qa? We renamed some sub-projects there.

@rjernst
Copy link
Member

rjernst commented Dec 11, 2018

This PR seems to have some unmerged other PRs in it, causing the diff to be around +35k lines...can you please re-ping for review when that is resolved?

@alpar-t
Copy link
Contributor Author

alpar-t commented Dec 12, 2018

@rjernst Not sure what you mean, the diff looks normal to me +85 −122 ( there are a lot f one line changes doing s/test/unitTest/ so 67 files ). Maybe there was some glitch ?

rjernst
rjernst approved these changes Dec 17, 2018
View reviewed changes
Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why github showed me a such a weird diff before; it looks fine now.

This looks fine to me. In addition to my one comment, I think the PR title needs to be changed, as it is only a side effect of the actual change: converting randomized testing to create a separate unitTest task instead of replacing the builtin test task.

buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingPlugin.groovy Outdated
it.enabled = false
RandomizedTestingTask unitTest = tasks.create('unitTest', RandomizedTestingTask)
unitTest.description = 'Runs unit tests with the randomized testing framework'
unitTest.include("**/*Tests.class")
Copy link
Member

@rjernst rjernst Dec 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think which tests are run should stay outside of RandomizedTesting code. It is still my intention to get this back into the randomizedtesting project, in which case the test naming patterns should not be assumed here.

@alpar-t alpar-t changed the title Remove gradle deprecation warnings Converting randomized testing to create a separate unitTest task instead of replacing the builtin test task Dec 18, 2018
@alpar-t alpar-t merged commit e9ef5bd into elastic:master Dec 19, 2018
@alpar-t alpar-t deleted the remove-gradle-deprecation-warnings branch December 19, 2018 06:25
alpar-t added a commit that referenced this pull request Dec 19, 2018
@alpar-t
Converting randomized testing to create a separate unitTest task inst…
f257360 
…ead of replacing the builtin test task (#36311)

- Create a separate unitTest task instead of Gradle's built in
- convert all configuration to use the new task
- the  built in task is now disabled
rjernst added a commit that referenced this pull request Dec 19, 2018
@rjernst
Fix tribe test to use unitTest for settings
c560a08 
relates #36311
rjernst added a commit that referenced this pull request Dec 19, 2018
@rjernst
Fix tribe test to use unitTest for settings
6861150 
relates #36311
dliappis added a commit to dliappis/elasticsearch that referenced this pull request Dec 21, 2018
@dliappis
Fix typo in unitTest task
639b991 
Fix the typo introdcued in elastic#36311 causing CI failures with the
FipsJvm.
@dliappis dliappis mentioned this pull request Dec 21, 2018
Merged
dliappis added a commit that referenced this pull request Dec 21, 2018
@dliappis
Fix typo in unitTest task (#36930)
8e97bf9 
Fix the typo introduced in #36311 causing CI failures with the
FipsJvm.
dliappis added a commit that referenced this pull request Dec 21, 2018
@dliappis
Fix typo in unitTest task (#36930)
879402f 
Fix the typo introduced in #36311 causing CI failures with the
FipsJvm.
tvernum pushed a commit that referenced this pull request Jan 2, 2019
@dliappis @tvernum
Fix typo in unitTest task (#36930)
5fc5c3a 
Fix the typo introduced in #36311 causing CI failures with the
FipsJvm.

(cherry picked from commit 8e97bf9)
@mark-vieira mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rjernst rjernst rjernst approved these changes

@nik9000 nik9000 Awaiting requested review from nik9000

@jasontedor jasontedor Awaiting requested review from jasontedor

Assignees

No one assigned

Labels

:Delivery/Build Build or test infrastructure >non-issue Team:Delivery Meta label for Delivery team v6.6.0 v7.0.0-beta1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Build] Gradle 5.0 deprecation: should not remove tasks

6 participants

@alpar-t @elasticmachine @jasontedor @rjernst @colings86 @mark-vieira