Skip to content
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.

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

KAFKA-18356 Explicitly setting up instrumentation for inline mocking (Java 21+) #18339

Open
wants to merge 4 commits into
base: trunk
Choose a base branch
from

Conversation

m1a2st
Copy link
Contributor

@m1a2st m1a2st commented Dec 28, 2024

Jira: https://issues.apache.org/jira/browse/KAFKA-18356

Starting from Java 21, the JDK restricts the ability of libraries to attach a Java agent to their own JVM. As a result, the inline-mock-maker might not be able to function without an explicit setup to enable instrumentation, and the JVM will always display a warning.

The mockito document

Test in my local
trunk:
CleanShot 2024-12-28 at 22 01 17@2x

this branch:
CleanShot 2024-12-28 at 22 00 15@2x

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@github-actions github-actions bot added triage PRs from the community build Gradle build or GitHub Actions labels Dec 28, 2024
build.gradle Outdated
mockitoAgent {
transitive = false
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Can we not do this once for all projects?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but not all projects need mockitoAgent, eg ':test-common:test-common-runtime', ':group-coordinator:group-coordinator-api', so I think its better for configuring it for separate projects

Copy link
Member

Choose a reason for hiding this comment

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

What's the downside of enabling it for those projects and how many such projects exist?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The downside is these projects need to implement libs.mockitoCore dependency which it doesn't need, and the following projects doesn't need it ':test-common:test-common-runtime', ':group-coordinator:group-coordinator-api', ':examples', ':generator', ':tools:tools-api', ':shell', ':streams:streams-scala', ':streams:examples', ':connect:transforms', ':connect:json', ':connect:mirror-client', ':connect:test-plugins'

Copy link
Member

@ijuma ijuma Dec 28, 2024

Choose a reason for hiding this comment

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

Could we enable it automatically if the mockitoCore dependency is added to a project?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a good idea, I will try for it :)

@github-actions github-actions bot added the small Small PRs label Dec 28, 2024
@github-actions github-actions bot removed the triage PRs from the community label Dec 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Gradle build or GitHub Actions ci-approved small Small PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants