Skip to content

Commit

Permalink
Fixing Unexpected method calls: HttpSession.invalidate (#2201)
Browse files Browse the repository at this point in the history 
## Summary
1. Why: The test failed sometimes with unexpected method calls.
2. What: The fix is preparing the test to accept invalidate method call too

## Expected Behavior
Tests are running without failure

## Actual Behavior
Tests are failing sometimes with unexpected method call.

## Steps to Reproduce
1. setup repeated run on e.g. `testCreateUserTask` in IDE
2. observe failure after multiple successful runs (for me it was failing after around 250 successful runs)


## Additional evidence
```
java.lang.AssertionError: On mock #2 (zero indexed): 
  Unexpected method calls:
    HttpSession.invalidate()
	at org.easymock.EasyMock.getAssertionError(EasyMock.java:2230)
	at org.easymock.EasyMock.verify(EasyMock.java:2058)
	at com.linkedin.kafka.cruisecontrol.servlet.UserTaskManagerTest.testCreateUserTask(UserTaskManagerTest.java:59)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
	at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
	at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
	at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.runTestClass(JUnitTestClassExecutor.java:112)
	at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.execute(JUnitTestClassExecutor.java:58)
	at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.execute(JUnitTestClassExecutor.java:40)
	at org.gradle.api.internal.tasks.testing.junit.AbstractJUnitTestClassProcessor.processTestClass(AbstractJUnitTestClassProcessor.java:60)
	at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.processTestClass(SuiteTestClassProcessor.java:52)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
	at org.gradle.internal.dispatch.ContextClassLoaderDispatch.dispatch(ContextClassLoaderDispatch.java:33)
	at org.gradle.internal.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:94)
	at com.sun.proxy.$Proxy5.processTestClass(Unknown Source)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker$2.run(TestWorker.java:176)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.executeAndMaintainThreadName(TestWorker.java:129)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:100)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:60)
	at org.gradle.process.internal.worker.child.ActionExecutionWorker.execute(ActionExecutionWorker.java:56)
	at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:113)
	at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:65)
	at worker.org.gradle.process.internal.worker.GradleWorkerMain.run(GradleWorkerMain.java:69)
	at worker.org.gradle.process.internal.worker.GradleWorkerMain.main(GradleWorkerMain.java:74)
```

## Categorization
- [x] bugfix
- [ ] new feature
- [ ] refactor
- [ ] CVE
- [ ] other
  • Loading branch information
@akatona84
akatona84 authored Oct 9, 2024
1 parent 6e45231 commit 2b81fb1
Showing 1 changed file with 15 additions and 18 deletions.
33 changes: 15 additions & 18 deletions ...e-control/src/test/java/com/linkedin/kafka/cruisecontrol/servlet/UserTaskManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,22 @@

public class UserTaskManagerTest {

private static HttpSession getMockHttpSession(long lastAccessedTimeReturn) {
HttpSession mockHttpSession = EasyMock.mock(HttpSession.class);
EasyMock.expect(mockHttpSession.getLastAccessedTime()).andReturn(lastAccessedTimeReturn).anyTimes();
mockHttpSession.invalidate();
EasyMock.expectLastCall().andVoid().anyTimes();
return mockHttpSession;
}

@Test
public void testCreateUserTask() throws Exception {
UUID testUserTaskId = UUID.randomUUID();

UserTaskManager.UuidGenerator mockUuidGenerator = EasyMock.mock(UserTaskManager.UuidGenerator.class);
EasyMock.expect(mockUuidGenerator.randomUUID()).andReturn(testUserTaskId).anyTimes();

HttpSession mockHttpSession = EasyMock.mock(HttpSession.class);
EasyMock.expect(mockHttpSession.getLastAccessedTime()).andReturn(100L).anyTimes();
HttpSession mockHttpSession = getMockHttpSession(100L);

HttpServletRequest mockHttpServletRequest1 = prepareServletRequest(mockHttpSession, null);

Expand Down Expand Up @@ -109,8 +116,7 @@ public void testSessionsShareUserTask() throws Exception {
UserTaskManager.UuidGenerator mockUuidGenerator = EasyMock.mock(UserTaskManager.UuidGenerator.class);
EasyMock.expect(mockUuidGenerator.randomUUID()).andReturn(testUserTaskId).anyTimes();

HttpSession mockHttpSession = EasyMock.mock(HttpSession.class);
EasyMock.expect(mockHttpSession.getLastAccessedTime()).andReturn(100L).anyTimes();
HttpSession mockHttpSession = getMockHttpSession(System.currentTimeMillis());

Map<String, String []> requestParams1 = new HashMap<>();
requestParams1.put("param", new String[]{"true"});
Expand Down Expand Up @@ -157,9 +163,7 @@ public void testAddStepsFutures() throws Exception {
UserTaskManager.UuidGenerator mockUuidGenerator = EasyMock.mock(UserTaskManager.UuidGenerator.class);
EasyMock.expect(mockUuidGenerator.randomUUID()).andReturn(testUserTaskId).anyTimes();

HttpSession mockHttpSession = EasyMock.mock(HttpSession.class);
// Change mock session's last access time to always return current time to avoid unintended recycling of session.
EasyMock.expect(mockHttpSession.getLastAccessedTime()).andReturn(System.currentTimeMillis()).anyTimes();
HttpSession mockHttpSession = getMockHttpSession(System.currentTimeMillis());

HttpServletRequest mockHttpServletRequest = prepareServletRequest(mockHttpSession, null);

Expand Down Expand Up @@ -192,9 +196,7 @@ public void testAddStepsFutures() throws Exception {

@Test
public void testCompletedTasks() throws Exception {
HttpSession mockHttpSession = EasyMock.mock(HttpSession.class);
EasyMock.expect(mockHttpSession.getLastAccessedTime()).andReturn(100L).anyTimes();
mockHttpSession.invalidate();
HttpSession mockHttpSession = getMockHttpSession(100L);

HttpServletRequest mockHttpServletRequest = prepareServletRequest(mockHttpSession, null);
UserTaskManager.UuidGenerator mockUuidGenerator = EasyMock.mock(UserTaskManager.UuidGenerator.class);
Expand Down Expand Up @@ -234,9 +236,7 @@ public void testExpireSession() throws Exception {
EasyMock.expect(mockUuidGenerator.randomUUID()).andReturn(testUserTaskId).anyTimes();

Time mockTime = new MockTime();
HttpSession mockHttpSession = EasyMock.mock(HttpSession.class);
EasyMock.expect(mockHttpSession.getLastAccessedTime()).andReturn(mockTime.milliseconds()).anyTimes();
mockHttpSession.invalidate();
HttpSession mockHttpSession = getMockHttpSession(100L);
HttpServletRequest mockHttpServletRequest = prepareServletRequest(mockHttpSession, null);

OperationFuture future = new OperationFuture("future");
Expand Down Expand Up @@ -265,8 +265,7 @@ public void testExpireSession() throws Exception {

@Test
public void testMaximumActiveTasks() throws Exception {
HttpSession mockHttpSession1 = EasyMock.mock(HttpSession.class);
EasyMock.expect(mockHttpSession1.getLastAccessedTime()).andReturn(100L).anyTimes();
HttpSession mockHttpSession1 = getMockHttpSession(100L);

HttpServletRequest mockHttpServletRequest1 = prepareServletRequest(mockHttpSession1, null);

Expand All @@ -285,9 +284,7 @@ public void testMaximumActiveTasks() throws Exception {
userTaskManager.getOrCreateUserTask(requestContext, uuid -> future, 0, true, null).get(0);
Assert.assertEquals(future, future1);
EasyMock.verify(mockHttpSession1, mockHttpServletRequest1, mockHttpServletResponse);
HttpSession mockHttpSession2 = EasyMock.mock(HttpSession.class);
EasyMock.expect(mockHttpSession2.getLastAccessedTime()).andReturn(100L).anyTimes();
EasyMock.replay(mockHttpSession2);
HttpSession mockHttpSession2 = getMockHttpSession(100L);
EasyMock.reset(mockHttpServletResponse);

HttpServletRequest mockHttpServletRequest2 = prepareServletRequest(mockHttpSession2, null, "/test2", Collections.emptyMap());
Expand Down

0 comments on commit 2b81fb1

Please sign in to comment.