fix: add missing EasyMock.verify calls before reset in unit tests by sameer-sde · Pull Request #2377 · linkedin/cruise-control

sameer-sde · 2026-06-01T20:13:28Z

Summary

Why: EasyMock's API contract requires verify() to be called before reset() on a mock, to ensure all expected interactions occurred before the mock's state is cleared. Several unit tests were resetting mocks without verifying them first.
What: Added the missing EasyMock.verify() calls before EasyMock.reset() in UserTaskManagerTest.java and KafkaCruiseControlServletEndpointTest.java, and added EasyMock.replay() after reset where needed to keep the mocks in a valid state for subsequent test cases.

Expected Behavior

All EasyMock mocks are verified before being reset, per EasyMock's contract, without breaking any existing test behavior.

Actual Behavior

Some tests called EasyMock.reset() without a preceding EasyMock.verify(), skipping validation of prior mock interactions.

Steps to Reproduce

Run UserTaskManagerTest and KafkaCruiseControlServletEndpointTest before this fix.
Observe that reset() calls occur without a prior verify(), meaning expected interactions on the mock were never validated.

Known Workarounds

None — this is a test-hygiene fix; it doesn't affect production behavior.

Additional evidence

All 6 UserTaskManagerTest tests pass.
KafkaCruiseControlServletEndpointTest.testUserTaskParameters is @Ignored upstream and remains skipped.

Categorization

This PR resolves #1408.

@ignored

…nkedin#1408) Add missing EasyMock.verify() calls before EasyMock.reset() in: - UserTaskManagerTest.java: added verify before each reset of mockHttpServletResponse, and added replay after reset with correct expectations to fix record-state issue - KafkaCruiseControlServletEndpointTest.java: added verify before each reset of MOCK_UUID_GENERATOR, MOCK_HTTP_SESSION, MOCK_HTTP_SERVLET_RESPONSE All 6 UserTaskManagerTest tests pass. KafkaCruiseControlServletEndpointTest is @ignored upstream and remains skipped. Closes linkedin#1408

kyguy · 2026-06-30T20:06:08Z

    List<UserTaskManager.UserTaskInfo> result2 = userTaskState.prepareResultList(parameters2);
    // Test Case 2 result
    Assert.assertEquals(3, result2.size());
+    EasyMock.verify(MOCK_UUID_GENERATOR, MOCK_HTTP_SESSION, MOCK_HTTP_SERVLET_RESPONSE);


From what I understand, the reset() call above clears the expectations made in the static block and no other expect() calls are made before this and the other verify() calls below. If that is true, wouldn't these verify() calls be no-ops? Are they intended to check something specific?

You're right, and thank you for pointing this out. After the first reset(), no new expect() calls are made on MOCK_UUID_GENERATOR, MOCK_HTTP_SESSION, or MOCK_HTTP_SERVLET_RESPONSE before the subsequent verify() calls for test cases 2–5. Since there are no expectations set, these verify() calls are effectively no-ops they trivially pass with an empty expectation set.
The only meaningful verify() is the one after result1, which validates the full setup from initializeServletRequests() and the static block (e.g., ensuring randomUUID() was called 6 times and setHeader was invoked as expected during populateUserTaskManager()).
I'll remove the redundant verify() calls for test cases 2–5 since they don't add any real verification value. Thanks for the catch!

kyguy · 2026-06-30T20:06:19Z

    List<UserTaskManager.UserTaskInfo> result3 = userTaskState.prepareResultList(parameters3);
    // Test Case 3 result
    Assert.assertEquals(2, result3.size());
+    EasyMock.verify(MOCK_UUID_GENERATOR, MOCK_HTTP_SESSION, MOCK_HTTP_SERVLET_RESPONSE);


Same as above

kyguy · 2026-06-30T20:06:28Z

    List<UserTaskManager.UserTaskInfo> result4 = userTaskState.prepareResultList(parameters4);
    // Test Case 4 result
    Assert.assertEquals(4, result4.size());
+    EasyMock.verify(MOCK_UUID_GENERATOR, MOCK_HTTP_SESSION, MOCK_HTTP_SERVLET_RESPONSE);


Same as above

kyguy · 2026-06-30T20:06:35Z

    List<UserTaskManager.UserTaskInfo> result5 = userTaskState2.prepareResultList(parameters5);
    // Test Case 5 result
    Assert.assertEquals(1, result5.size());
+    EasyMock.verify(MOCK_UUID_GENERATOR, MOCK_HTTP_SESSION, MOCK_HTTP_SERVLET_RESPONSE);


Same as above

kyguy · 2026-06-30T20:42:54Z

    HttpServletRequest mockHttpServletRequest2 = prepareServletRequest(mockHttpSession, testUserTaskId.toString());
    EasyMock.reset(mockHttpServletResponse);
+    requestContext.setHeader(EasyMock.capture(userTaskHeader), EasyMock.capture(userTaskHeaderValue));
+    EasyMock.replay(mockHttpServletResponse);


Isn't the header name the same across all the tests cases? If so, do these lines add any additional coverage beyond what we have in line 52?

Good catch! The header name is always the same constant (USER_TASK_HEADER_NAME) across all sub-cases, so the capture doesn't assert anything new about the header name itself.
However, the reset() + setHeader capture + replay() blocks are necessary for correctness, not just coverage. After reset(mockHttpServletResponse), the mock reverts to record state. The subsequent getOrCreateUserTask() call internally invokes setHeader on the response mock without re-entering replay mode via replay(mockHttpServletResponse), this would result in an unexpected invocation error and break the test.
So these blocks are a required fix to keep the mock in a valid state for each sub-case. I agree the header name assertion is redundant I can simplify by using EasyMock.anyString() without the capture if you prefer, since we're not asserting anything new about it. Happy to update

kyguy · 2026-06-30T20:43:02Z


    EasyMock.reset(mockHttpServletResponse);
+    requestContext.setHeader(EasyMock.capture(userTaskHeader), EasyMock.capture(userTaskHeaderValue));
+    EasyMock.replay(mockHttpServletResponse);


Same as above

kyguy · 2026-06-30T20:43:10Z


    EasyMock.reset(mockHttpServletResponse);
+    requestContext2.setHeader(EasyMock.capture(userTaskHeader), EasyMock.capture(userTaskHeaderValue));
+    EasyMock.replay(mockHttpServletResponse);


Same as above

mhratson · 2026-07-16T17:36:05Z

Please follow PR template format.

sameer-sde · 2026-07-18T09:45:16Z

Thanks @mhratson, I've reformatted the PR description to follow the template.

Copilot

Pull request overview

This PR updates EasyMock-based unit tests to follow EasyMock’s contract by ensuring mocks are verified before being reset, and (where applicable) brought back into a valid state for subsequent interactions.

Changes:

Added EasyMock.verify(...) before EasyMock.reset(...) in KafkaCruiseControlServletEndpointTest.
Added additional setHeader(...) expectations and EasyMock.replay(...) calls after reset(...) in UserTaskManagerTest to keep the response mock usable across multiple test cases.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 9 comments.

File	Description
cruise-control/src/test/java/com/linkedin/kafka/cruisecontrol/servlet/UserTaskManagerTest.java	Adjusts response-mock reset/replay usage while exercising multiple `getOrCreateUserTask` scenarios.
cruise-control/src/test/java/com/linkedin/kafka/cruisecontrol/servlet/KafkaCruiseControlServletEndpointTest.java	Adds verify-before-reset calls around multiple “test cases” within a single ignored test method.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

    EasyMock.reset(mockHttpServletResponse);
+    requestContext.setHeader(EasyMock.capture(userTaskHeader), EasyMock.capture(userTaskHeaderValue));
+    EasyMock.replay(mockHttpServletResponse);


@@ -75,6 +75,8 @@ public void testCreateUserTask() throws Exception {

    HttpServletRequest mockHttpServletRequest2 = prepareServletRequest(mockHttpSession, testUserTaskId.toString());
    EasyMock.reset(mockHttpServletResponse);


@@ -85,6 +87,8 @@ public void testCreateUserTask() throws Exception {
    Assert.assertEquals(future, future3);

    EasyMock.reset(mockHttpServletResponse);


@@ -100,6 +104,8 @@ public void testCreateUserTask() throws Exception {
    Assert.assertEquals(future4, future);

    EasyMock.reset(mockHttpServletResponse);


+    EasyMock.verify(MOCK_UUID_GENERATOR, MOCK_HTTP_SESSION, MOCK_HTTP_SERVLET_RESPONSE);
    EasyMock.reset(MOCK_UUID_GENERATOR, MOCK_HTTP_SESSION, MOCK_HTTP_SERVLET_RESPONSE);


+    EasyMock.verify(MOCK_UUID_GENERATOR, MOCK_HTTP_SESSION, MOCK_HTTP_SERVLET_RESPONSE);
    EasyMock.reset(MOCK_UUID_GENERATOR, MOCK_HTTP_SESSION, MOCK_HTTP_SERVLET_RESPONSE);


+    EasyMock.verify(MOCK_UUID_GENERATOR, MOCK_HTTP_SESSION, MOCK_HTTP_SERVLET_RESPONSE);
    EasyMock.reset(MOCK_UUID_GENERATOR, MOCK_HTTP_SESSION, MOCK_HTTP_SERVLET_RESPONSE);


+    EasyMock.verify(MOCK_UUID_GENERATOR, MOCK_HTTP_SESSION, MOCK_HTTP_SERVLET_RESPONSE);
    EasyMock.reset(MOCK_UUID_GENERATOR, MOCK_HTTP_SESSION, MOCK_HTTP_SERVLET_RESPONSE);


+    EasyMock.verify(MOCK_UUID_GENERATOR, MOCK_HTTP_SESSION, MOCK_HTTP_SERVLET_RESPONSE);
    EasyMock.reset(MOCK_UUID_GENERATOR, MOCK_HTTP_SESSION, MOCK_HTTP_SERVLET_RESPONSE);


kyguy reviewed Jun 30, 2026

View reviewed changes

mhratson requested a review from Copilot July 21, 2026 18:08

Copilot started reviewing on behalf of mhratson July 21, 2026 18:08 View session

Copilot AI reviewed Jul 21, 2026

View reviewed changes

Navigation Menu

Uh oh!

fix: add missing EasyMock.verify calls before reset in unit tests - #2377

fix: add missing EasyMock.verify calls before reset in unit tests #2377
sameer-sde wants to merge 1 commit into
linkedin:mainfrom
sameer-sde:fix/1408-add-missing-easymock-verify

sameer-sde commented Jun 1, 2026 •

edited

Loading

Uh oh!

kyguy Jun 30, 2026

Uh oh!

sameer-sde Jun 30, 2026

Uh oh!

kyguy Jun 30, 2026

Uh oh!

kyguy Jun 30, 2026

Uh oh!

kyguy Jun 30, 2026

Uh oh!

kyguy Jun 30, 2026

Uh oh!

sameer-sde Jun 30, 2026

Uh oh!

kyguy Jun 30, 2026

Uh oh!

kyguy Jun 30, 2026

Uh oh!

mhratson commented Jul 16, 2026

Uh oh!

sameer-sde commented Jul 18, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		@@ -75,6 +75,8 @@ public void testCreateUserTask() throws Exception {

		HttpServletRequest mockHttpServletRequest2 = prepareServletRequest(mockHttpSession, testUserTaskId.toString());
		EasyMock.reset(mockHttpServletResponse);

		@@ -85,6 +87,8 @@ public void testCreateUserTask() throws Exception {
		Assert.assertEquals(future, future3);

		EasyMock.reset(mockHttpServletResponse);

		@@ -100,6 +104,8 @@ public void testCreateUserTask() throws Exception {
		Assert.assertEquals(future4, future);

		EasyMock.reset(mockHttpServletResponse);

		EasyMock.verify(MOCK_UUID_GENERATOR, MOCK_HTTP_SESSION, MOCK_HTTP_SERVLET_RESPONSE);
		EasyMock.reset(MOCK_UUID_GENERATOR, MOCK_HTTP_SESSION, MOCK_HTTP_SERVLET_RESPONSE);

Uh oh!

Conversation

sameer-sde commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Expected Behavior

Actual Behavior

Steps to Reproduce

Known Workarounds

Additional evidence

Categorization

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mhratson commented Jul 16, 2026

Uh oh!

sameer-sde commented Jul 18, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sameer-sde commented Jun 1, 2026 •

edited

Loading