Skip to content

Conversation

@breskeby
Copy link
Contributor

@breskeby breskeby commented Sep 23, 2021

  • This fixes having transitive compile only dependencies on the classpath for consumers of test artifacts
  • We remove compile dependencies from the testArtifact configuration dependencies here as this leaks implementation and compile only deps to consuming projects

@breskeby breskeby self-assigned this Sep 23, 2021
@breskeby breskeby added :Delivery/Build Build or test infrastructure >enhancement Team:Delivery Meta label for Delivery team v7.16.0 v8.0.0 labels Sep 23, 2021
- This fixes having transitive compile only dependencies on the classpath for consumers of test artifacts
@breskeby breskeby force-pushed the cleanup-test-artifact-compileclasspath branch from f99fe3b to ff977d0 Compare September 27, 2021 17:43
@breskeby breskeby marked this pull request as ready for review September 27, 2021 20:21
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-delivery (Team:Delivery)

Copy link
Contributor

@mark-vieira mark-vieira left a comment

Choose a reason for hiding this comment

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

Does this mean we now don't get any transitive dependencies from test artifacts?

@breskeby
Copy link
Contributor Author

@mark-vieira basically all elasticsearch transitive modules and their api dependencies end up on the classpath now instead of the whole compile classpath that has leaked compileOnly and implementation configurations too.

here's an example:
Screenshot 2021-09-28 at 10 47 20

Copy link
Contributor

@mark-vieira mark-vieira left a comment

Choose a reason for hiding this comment

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

LGTM, although I think we might want to make some test dependencies api intead of implementation to reduce the need for consumers to have to declare those transitive dependencies that are obviously needed at compile time.

dependencies {
testImplementation project(xpackModule('core'))
testImplementation(testArtifact(project(xpackModule('core'))))
testImplementation project(':test:framework')
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering if instances like this should really be testApi since the intent is that folks that consume this test artifact will also obviously need the test framework to compile against this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah this makes kind of sense. I'd like to revisit the api of our testFixtures in a separate commit.

Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW, @dakrone already hit this issue, where he pulled in this change and then had too add a bunch of transitive dependencies to get things working again.

@breskeby
Copy link
Contributor Author

This fixes having transitive compile only dependencies on the classpath for consumers of test artifacts
We remove compile dependencies from the testArtifact configuration dependencies here as this leaks implementation and compile only deps to consuming projects

@breskeby breskeby merged commit 4e845aa into elastic:master Sep 29, 2021
breskeby added a commit to breskeby/elasticsearch that referenced this pull request Sep 29, 2021
- This fixes having transitive compile only dependencies on the classpath for consumers of test artifacts
- We remove compile dependencies from the testArtifact configuration dependencies here as this leaks implementation and compile only deps to consuming projects
breskeby added a commit that referenced this pull request Sep 29, 2021
- This fixes having transitive compile only dependencies on the classpath for consumers of test artifacts
- We remove compile dependencies from the testArtifact configuration dependencies here as this leaks implementation and compile only deps to consuming projects
@breskeby breskeby deleted the cleanup-test-artifact-compileclasspath branch September 29, 2021 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Delivery/Build Build or test infrastructure >enhancement Team:Delivery Meta label for Delivery team v7.16.0 v8.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants