Attach Mockito as an agent#12788
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
scholzj
left a comment
There was a problem hiding this comment.
Thanks for the PR. Can you please explain why does it need to be done on a module by module basis? And why only in some of the modules?
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #12788 +/- ##
============================================
- Coverage 75.16% 75.06% -0.10%
+ Complexity 6460 6433 -27
============================================
Files 346 346
Lines 24336 24279 -57
Branches 3123 3111 -12
============================================
- Hits 18292 18225 -67
- Misses 4809 4824 +15
+ Partials 1235 1230 -5 🚀 New features to boost your workflow:
|
|
As illustrated in Mockito's documentation
As a result, we have to explicitly set up the mockito-core as a Java agent.
I tried to do it in root However, when Maven is doing variable Interpolate, the strimzi-kafka-operator/pom.xml Lines 1017 to 1025 in 79fb916 So the modules with the mockito-core as dependency (modules that are really using mockito) should all be updated. That way the strimzi-kafka-operator/kafka-init/pom.xml Lines 75 to 78 in 79fb916 Of course we keep the Mockito config globally. However, the will introduce more file changes due to dependency injection, which i think it's not that necessary. |
|
@bboyleonp666 Ok, I guess having the agent part onlyin some modules makes sense. But why did you removed the |
|
Thanks for the question. Let me explain the original motivation and what I found after re-analyzing. First, it helps to split the modules by whether they have a module-level surefire
I was trying everything in a bundle, which misleads myself to think dropping the Which works right after removing it from The root cause is how Maven resolves ExperimentsI ran experiments across 3 scenarios on 2 representative modules:
The scenarios differ in what
Files checked
Why
|
|
@bboyleonp666 That is a rather long and unclear answer for a simple question. Strictly speaking, I do not think the coverage profile is used at all. But I do not understand why it needs to be touched in this PR as part of this work. |
|
I see your point. Back to the issue itself, there's no need to do that. Let me revert that change. |
b532fdc to
7a60eff
Compare
|
/gha run pipeline=acceptance |
|
⏳ System test verification started: link The following 2 job(s) will be executed:
Tests will start after successful build completion. |
|
❌ System test verification failed: link |
I think it's clearly verbosity coming from AI tooling which was not cleaned at all. @bboyleonp666 if you are using AI (which looks pretty clear to me), given our AI policy https://github.com/strimzi/governance/blob/main/AI_POLICY.md in the governance you should disclose it clearly in the PR description. Thanks. |
|
@ppatierno thanks for pointing that out. I've added a disclosure in the PR description. |
Signed-off-by: bboyleonp666 <bboyleonp@gmail.com>
Signed-off-by: bboyleonp666 <bboyleonp@gmail.com>
|
❌ System test verification failed: link |
|
I think the system test issue is related to this PR and its changes in some way. |
|
I tried to run system tests for acceptance group locally and it behaves similarly. It seems that I didn't include the latest change that supports kafka v4.2.1 as in kafka-versions.yaml, which induces the resources never ready issue. |
7a60eff to
d6867b2
Compare
|
/gha run pipeline=acceptance |
|
⏳ System test verification started: link The following 2 job(s) will be executed:
Tests will start after successful build completion. |
|
I think it would be pretty weird if rebased fixed this because the tests run against a merge. So I think there will be some other problem there. |
|
I think you are totally right. I just check the logs of the running system tests. It fails with the same reason again. All waiting for Kakfa/cluster Investigating in that direction. Do you have any guess that might help? |
|
I'm afraid I have no idea what the problem is. Especially if it seems to be working for you locally. :-( Maybe checking the logs once the tests are finished could help to understand if the Kafka cluster did not deployed at all or what the problem was. |
|
❌ System test verification failed: link |
Type of change
Description
This PR is to close #12749. As the future JDK no longer supports the self-attaching agent. To deal with this warning and support future upgrade, this PR explicitly declares
mockito-coreas a java agent on a module-by-module basis.Checklist
Please go through this checklist and make sure all applicable tasks have been done
Disclosure
I am using Claude Code to help me dig into the code as well as generating tables to answer questions. However, the ideas and the descriptions are all typed by me manually.