Onboarding Unit Testing with Mocking Framework for Kruize

[pinkygupta-hub]

Description

Onboarding Unit Testing with Mocking Framework for Kruize, added required dependencies in pom and added sample test cases with guidelines.

Fixes # (issue)

Type of change

• Bug fix
• New feature
• Docs update
• Breaking change (What changes might users need to make in their application due to this PR?)
• Requires DB changes

How has this been tested?

Please describe the tests that were run to verify your changes and steps to reproduce. Please specify any test configuration required.

• New Test X
• Functional testsuite

Test Configuration

• Kubernetes clusters tested on: minkube

Checklist 🎯

• Followed coding guidelines
• Comments added
• Dependent changes merged
• Documentation updated
• Tests added or updated

Additional information

Include any additional information such as links, test results, screenshots here
Successful test cases, command : mvn clean test

Summary by Sourcery

Introduce a mocking-based unit testing setup for Kruize and configure the build to run JUnit 5 tests reliably.

Build:

• Update JUnit and Maven Surefire plugin versions and configure test includes and runtime options for JUnit 5 and Mockito.

Tests:

• Add example Mockito-based unit tests for BulkJobManager to validate experiment name framing logic and serve as a template for future tests.

[sourcery-ai]

Reviewer's Guide

Introduces a Mockito-based JUnit 5 unit test for BulkJobManager’s experiment-name framing logic and modernizes the Maven test stack (JUnit 5 BOM, Mockito, and Surefire) to support ongoing unit testing in the Kruize analyzer module.

Sequence diagram for BulkJobManager experiment name unit test

sequenceDiagram
actor Developer
participant Maven
participant Surefire
participant JUnit5
participant Mockito
participant BulkJobManagerTest
participant BulkJobManager
participant DataSourceMock
participant WorkloadMetadataMock
participant LabelProviderMock

Developer->>Maven: run mvn clean test
Maven->>Surefire: execute tests
Surefire->>JUnit5: discover and run tests
JUnit5->>BulkJobManagerTest: construct test instance
JUnit5->>Mockito: initialize mocks
Mockito-->>BulkJobManagerTest: inject DataSourceMock, WorkloadMetadataMock, LabelProviderMock

JUnit5->>BulkJobManagerTest: invoke testConstructExperimentNameWithLabels
BulkJobManagerTest->>DataSourceMock: configure experiment data
BulkJobManagerTest->>WorkloadMetadataMock: configure workload metadata
BulkJobManagerTest->>LabelProviderMock: configure labels

BulkJobManagerTest->>BulkJobManager: constructExperimentName(DataSourceMock, WorkloadMetadataMock, LabelProviderMock)
BulkJobManager->>DataSourceMock: getExperimentSource()
DataSourceMock-->>BulkJobManager: experimentSource
BulkJobManager->>WorkloadMetadataMock: getWorkloadName()
WorkloadMetadataMock-->>BulkJobManager: workloadName
BulkJobManager->>LabelProviderMock: getLabelValues()
LabelProviderMock-->>BulkJobManager: labelValues
BulkJobManager-->>BulkJobManagerTest: experimentName

BulkJobManagerTest->>BulkJobManagerTest: assert experimentName
BulkJobManagerTest-->>JUnit5: success
JUnit5-->>Surefire: report test success
Surefire-->>Maven: build success
Maven-->>Developer: tests passed

Loading

Class diagram for BulkJobManager unit test with mocking

classDiagram
class BulkJobManager {
  +constructExperimentName(DataSource dataSource, WorkloadMetadata workloadMetadata, LabelProvider labelProvider) String
}

class BulkJobManagerTest {
  +setUp()
  +testConstructExperimentNameWithLabels()
}

class DataSource
class WorkloadMetadata
class LabelProvider

BulkJobManagerTest --> BulkJobManager : tests
BulkJobManager --> DataSource : uses
BulkJobManager --> WorkloadMetadata : uses
BulkJobManager --> LabelProvider : uses

Loading

File-Level Changes

Change	Details	Files
Modernize JUnit 5 setup and add Mockito dependencies for unit testing.	Replace junit-jupiter-engine 5.3.1 with junit-jupiter 5.10.2 and restrict it to the test scope. Add mockito-core and mockito-junit-jupiter 5.8.0 as test-scoped dependencies to enable mocking and JUnit 5 integration.	pom.xml
Update Maven Surefire plugin configuration for modern JUnit 5 execution and stricter test behavior.	Bump maven-surefire-plugin from 2.22.0 to 3.1.2 to improve JUnit 5 support. Configure includes pattern to only run *Test.java classes. Enable failIfNoTests to ensure missing tests fail the build. Set argLine to enable ByteBuddy experimental features needed for Mockito.	pom.xml
Add a mocked BulkJobManager unit test that demonstrates the project’s mocking/testing conventions.	Introduce BulkJobManagerMockedTest that focuses on testing BulkJobManager as a single unit with mocked collaborators. Use Mockito to mock BulkInput, BulkJobStatus, and data source metadata objects, defining deterministic behavior through when(...) stubbing. Validate frameExperimentName behavior both with and without labels, asserting that the composed experiment name matches the configured KruizeDeploymentInfo.experiment_name_format. Document testing principles (single class under test, mocking only direct dependencies, no global side effects, Given–When–Then structure, deterministic assertions) as a template for future tests.	src/test/java/com/autotune/analyzer/workerimpl/BulkJobManagerTest.java

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've found 2 issues, and left some high level feedback:

• The tests mutate the static KruizeDeploymentInfo.experiment_name_format field without restoring its original value, which can leak configuration to other tests—consider capturing the original value and resetting it in an @AfterEach (or using a helper) to keep tests isolated.
• In BulkJobManagerMockedTest, some mocks (e.g., BulkJobStatus jobStatus) appear unused or only minimally exercised; trimming unused mocks or stubbing only what BulkJobManager actually consumes will keep the test focused and easier to maintain.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The tests mutate the static `KruizeDeploymentInfo.experiment_name_format` field without restoring its original value, which can leak configuration to other tests—consider capturing the original value and resetting it in an `@AfterEach` (or using a helper) to keep tests isolated.
- In `BulkJobManagerMockedTest`, some mocks (e.g., `BulkJobStatus jobStatus`) appear unused or only minimally exercised; trimming unused mocks or stubbing only what `BulkJobManager` actually consumes will keep the test focused and easier to maintain.

## Individual Comments

### Comment 1
<location> `pom.xml:293` </location>
<code_context>
+                        <include>**/*Test.java</include>
+                    </includes>
+                    <failIfNoTests>true</failIfNoTests>
+                    <argLine>-Dnet.bytebuddy.experimental=true</argLine>
+                </configuration>
             </plugin>
</code_context>

<issue_to_address>
**suggestion (bug_risk):** Hardcoding `argLine` may override or conflict with other plugins (e.g., JaCoCo) that also set it.

This will overwrite any `argLine` configured by other plugins (e.g. `jacoco-maven-plugin`), which can disable coverage or other JVM options. Prefer appending to the existing value (e.g. `${argLine} -Dnet.bytebuddy.experimental=true`) or using a composable property (such as `${surefireArgLine}`) instead.

```suggestion
                    <argLine>${argLine} -Dnet.bytebuddy.experimental=true</argLine>
```
</issue_to_address>

### Comment 2
<location> `pom.xml:289-290` </location>
<code_context>
-                <version>2.22.0</version>
+                <version>3.1.2</version>
+                <configuration>
+                    <includes>
+                        <include>**/*Test.java</include>
+                    </includes>
+                    <failIfNoTests>true</failIfNoTests>
</code_context>

<issue_to_address>
**question (testing):** The surefire include pattern may unintentionally skip valid tests with other naming conventions.

This change will exclude classes like `*Tests.java`, `*IT.java`, or other existing naming patterns from the test run. If the module uses mixed conventions or contains integration tests, please either broaden the include patterns (e.g., add `**/*Tests.java`, `**/*IT.java`) or rely on surefire’s defaults unless there’s a strong reason to restrict them.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[chandrams]

@pinkygupta-hub I see these warnings when I run this, can you take a look at this

[INFO] Running com.autotune.analyzer.workerimpl.BulkJobManagerMockedTest
WARNING: A Java agent has been loaded dynamically (/home/chandrams/.m2/repository/net/bytebuddy/byte-buddy-agent/1.14.10/byte-buddy-agent-1.14.10.jar)
WARNING: If a serviceability tool is in use, please run with -XX:+EnableDynamicAgentLoading to hide this warning
WARNING: If a serviceability tool is not in use, please run with -Djdk.instrument.traceUsage for more information
WARNING: Dynamic loading of agents will be disallowed by default in a future release
OpenJDK 64-Bit Server VM warning: Sharing is only supported for boot loader classes because bootstrap classpath has been appended
WARNING: A terminally deprecated method in sun.misc.Unsafe has been called
WARNING: sun.misc.Unsafe::objectFieldOffset has been called by net.bytebuddy.dynamic.loading.ClassInjector$UsingUnsafe$Dispatcher$CreationAction (file:/home/chandrams/.m2/repository/net/bytebuddy/byte-buddy/1.14.9/byte-buddy-1.14.9.jar)

[bharathappali]

LGTM

[pinkygupta-hub]

@pinkygupta-hub I see these warnings when I run this, can you take a look at this

[INFO] Running com.autotune.analyzer.workerimpl.BulkJobManagerMockedTest
WARNING: A Java agent has been loaded dynamically (/home/chandrams/.m2/repository/net/bytebuddy/byte-buddy-agent/1.14.10/byte-buddy-agent-1.14.10.jar)
WARNING: If a serviceability tool is in use, please run with -XX:+EnableDynamicAgentLoading to hide this warning
WARNING: If a serviceability tool is not in use, please run with -Djdk.instrument.traceUsage for more information
WARNING: Dynamic loading of agents will be disallowed by default in a future release
OpenJDK 64-Bit Server VM warning: Sharing is only supported for boot loader classes because bootstrap classpath has been appended
WARNING: A terminally deprecated method in sun.misc.Unsafe has been called
WARNING: sun.misc.Unsafe::objectFieldOffset has been called by net.bytebuddy.dynamic.loading.ClassInjector$UsingUnsafe$Dispatcher$CreationAction (file:/home/chandrams/.m2/repository/net/bytebuddy/byte-buddy/1.14.9/byte-buddy-1.14.9.jar)

Thanks for pointing this out. These warnings are coming from Mockito/ByteBuddy when running on newer Java versions. They are related to dynamic agent loading and internal APIs, not our implementation, and don’t affect test execution. This is expected behavior and should go away once dependencies are updated to fully support the current JDK. We are already using Mockito 5.8, which is a recent version.

[chandrams]

I see 5.21.0 available, can you update to the latest versions and test it