From 70e3b2a6fa4f860c01c7a2a7703714c54ddca9c5 Mon Sep 17 00:00:00 2001 From: Brendan Burns Date: Thu, 10 Feb 2022 19:06:01 +0000 Subject: [PATCH 1/3] Deflake another test. --- .../controller/DefaultControllerTest.java | 6 +-- .../LeaderElectingControllerTest.java | 50 +++++++++++++------ 2 files changed, 38 insertions(+), 18 deletions(-) diff --git a/extended/src/test/java/io/kubernetes/client/extended/controller/DefaultControllerTest.java b/extended/src/test/java/io/kubernetes/client/extended/controller/DefaultControllerTest.java index f7d4232cac..6746998315 100644 --- a/extended/src/test/java/io/kubernetes/client/extended/controller/DefaultControllerTest.java +++ b/extended/src/test/java/io/kubernetes/client/extended/controller/DefaultControllerTest.java @@ -61,7 +61,7 @@ public void tearDown() throws Exception {} @Mock private Reconciler mockReconciler; - @Test + @Test(timeout = 90000) public void testStartingStoppingController() throws InterruptedException { DefaultController testController = new DefaultController("", mockReconciler, workQueue); @@ -102,7 +102,7 @@ public Object answer(InvocationOnMock invocation) { verify(mockReconciler, times(0)).reconcile(request2); } - @Test + @Test(timeout = 90000) public void testControllerWontStartBeforeReady() throws InterruptedException { Request request1 = new Request("test1"); @@ -139,7 +139,7 @@ public Object answer(InvocationOnMock invocation) { verify(mockReconciler, times(1)).reconcile(request1); } - @Test + @Test(timeout = 90000) public void testControllerKeepsWorkingWhenReconcilerAbortsWithRuntimeException() throws InterruptedException { AtomicBoolean aborts = new AtomicBoolean(true); diff --git a/extended/src/test/java/io/kubernetes/client/extended/controller/LeaderElectingControllerTest.java b/extended/src/test/java/io/kubernetes/client/extended/controller/LeaderElectingControllerTest.java index bce58270ff..a4c911b54e 100644 --- a/extended/src/test/java/io/kubernetes/client/extended/controller/LeaderElectingControllerTest.java +++ b/extended/src/test/java/io/kubernetes/client/extended/controller/LeaderElectingControllerTest.java @@ -15,7 +15,6 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.doAnswer; -import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -27,6 +26,7 @@ import io.kubernetes.client.openapi.ApiException; import java.net.HttpURLConnection; import java.time.Duration; +import java.util.concurrent.Semaphore; import java.util.concurrent.atomic.AtomicReference; import org.junit.Test; import org.junit.runner.RunWith; @@ -40,22 +40,15 @@ public class LeaderElectingControllerTest { @Mock private Lock mockLock; - private final int stepCooldownIntervalInMillis = 2000; - - private void cooldown() { - try { - Thread.sleep(stepCooldownIntervalInMillis); - } catch (InterruptedException e) { - e.printStackTrace(); - } - } - - @Test - public void testLeaderElectingController() throws ApiException { + @Test(timeout = 90000) + public void testLeaderElectingController() throws ApiException, InterruptedException { AtomicReference record = new AtomicReference<>(); record.set(new LeaderElectionRecord()); + Semaphore latch = new Semaphore(2); + Semaphore controllerLatch = new Semaphore(2); + when(mockLock.identity()).thenReturn("foo"); when(mockLock.get()) .thenThrow( @@ -65,12 +58,35 @@ public void testLeaderElectingController() throws ApiException { doAnswer( invocationOnMock -> { record.set(invocationOnMock.getArgument(0)); + latch.release(); return true; }) .when(mockLock) .create(any()); - doReturn(false).when(mockLock).update(any()); + doAnswer( + invocationOnMock -> { + latch.release(); + return false; + }) + .when(mockLock) + .update(any()); + + doAnswer( + invocationOnMock -> { + controllerLatch.release(); + return null; + }) + .when(mockController) + .run(); + + doAnswer( + invocationOnMock -> { + controllerLatch.release(); + return null; + }) + .when(mockController) + .shutdown(); LeaderElectingController leaderElectingController = new LeaderElectingController( @@ -82,14 +98,18 @@ public void testLeaderElectingController() throws ApiException { Duration.ofMillis(100))), mockController); + latch.acquire(2); + controllerLatch.acquire(2); + Thread controllerThread = new Thread(leaderElectingController::run); controllerThread.start(); - cooldown(); + latch.acquire(2); controllerThread.interrupt(); verify(mockLock, times(1)).create(any()); verify(mockLock, atLeastOnce()).update(any()); + controllerLatch.acquire(2); verify(mockController, times(1)).run(); verify(mockController, times(1)).shutdown(); } From 67b5f2ed41dc5851f8bb8418412dc91975d9bd6e Mon Sep 17 00:00:00 2001 From: Brendan Burns Date: Wed, 1 Dec 2021 10:21:27 -0800 Subject: [PATCH 2/3] Add macos to the matrix build. --- .github/workflows/maven.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 5868135e76..ac4653ee75 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -28,7 +28,7 @@ jobs: matrix: # Test against the LTS Java versions. TODO: add JDK18 when it becomes available. java: [ 8.0.x, 11.0.x, 17.0.x ] - os: [ windows-latest, ubuntu-latest ] + os: [ macos-latest, windows-latest, ubuntu-latest ] runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v2.4.0 From e4cca45b4e9cc785b01ec0e93c0438b343743676 Mon Sep 17 00:00:00 2001 From: Brendan Burns Date: Fri, 11 Feb 2022 18:59:41 +0000 Subject: [PATCH 3/3] Deflake LeaderElectionTest --- .../leaderelection/LeaderElectionTest.java | 47 ++++++++++++++++--- 1 file changed, 40 insertions(+), 7 deletions(-) diff --git a/extended/src/test/java/io/kubernetes/client/extended/leaderelection/LeaderElectionTest.java b/extended/src/test/java/io/kubernetes/client/extended/leaderelection/LeaderElectionTest.java index ca5c302eba..a203c8d416 100644 --- a/extended/src/test/java/io/kubernetes/client/extended/leaderelection/LeaderElectionTest.java +++ b/extended/src/test/java/io/kubernetes/client/extended/leaderelection/LeaderElectionTest.java @@ -201,24 +201,29 @@ public void testLeaderElectionWithRenewDeadline() throws InterruptedException { List electionHistory = new ArrayList<>(); List leadershipHistory = new ArrayList<>(); + CountDownLatch testLockAccessLatch = new CountDownLatch(9); MockResourceLock mockLock = new MockResourceLock("mock"); mockLock.renewCountMax = 3; mockLock.onCreate = record -> { electionHistory.add("create record"); leadershipHistory.add("get leadership"); + testLockAccessLatch.countDown(); }; mockLock.onUpdate = record -> { electionHistory.add("update record"); + testLockAccessLatch.countDown(); }; mockLock.onChange = record -> { electionHistory.add("change record"); + testLockAccessLatch.countDown(); }; mockLock.onTryUpdate = record -> { electionHistory.add("try update record"); + testLockAccessLatch.countDown(); }; LeaderElectionConfig leaderElectionConfig = new LeaderElectionConfig(); @@ -244,18 +249,16 @@ record -> { }); testLeaderElectionLatch.await(10, SECONDS); + testLockAccessLatch.await(10, SECONDS); - assertHistory( + assertWildcardHistory( electionHistory, "create record", - "try update record", + "try update record+", "update record", - "try update record", + "try update record+", "update record", - "try update record", - "try update record", - "try update record", - "try update record"); + "try update record+"); assertHistory(leadershipHistory, "get leadership", "start leading", "stop leading"); } @@ -274,6 +277,36 @@ private void assertHistory(List history, String... expected) { } } + // assertWildcardHistory allows for an arbitrary number of repeated entries for an + // comparison with a '+' suffix. This allows for a semantic rather than literal + // comparison to avoid issues of timing. + private void assertWildcardHistory(List history, String... expected) { + Assert.assertNotNull(expected); + Assert.assertNotNull(history); + + // TODO: This code is too complicated and a little bit buggy, but it works + // for the current limited use case. Clean this up! + int expectedIx = 0; + for (int index = 0; index < history.size(); ++index) { + String compare = expected[expectedIx]; + if (compare.endsWith("+")) { + compare = compare.substring(0, compare.length() - 1); + if (!history.get(index).equals(compare)) { + expectedIx++; + compare = expected[expectedIx]; + expectedIx++; + } + } else { + expectedIx++; + } + Assert.assertEquals( + String.format( + "Not equal at index %d, expected %s, got %s", index, compare, history.get(index)), + compare, + history.get(index)); + } + } + @Test public void testLeaderElectionCaptureException() throws ApiException, InterruptedException { RuntimeException expectedException = new RuntimeException("noxu");