From 1a622e707fd160080ea314a53a5630d06e6e2875 Mon Sep 17 00:00:00 2001 From: Ricardo Martin Date: Tue, 21 Mar 2023 08:30:42 +0100 Subject: [PATCH] Flaky tests org.keycloak.testsuite.federation.sync.SyncFederationTest (#19095) Closes: https://github.com/keycloak/keycloak/issues/17430 Closes: https://github.com/keycloak/keycloak/issues/17431 --- ...yncDummyUserFederationProviderFactory.java | 25 ++++++++----- .../federation/sync/SyncFederationTest.java | 36 ++++++++++++------- 2 files changed, 41 insertions(+), 20 deletions(-) diff --git a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/federation/sync/SyncDummyUserFederationProviderFactory.java b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/federation/sync/SyncDummyUserFederationProviderFactory.java index d01e03ee3d..8da5a958a5 100644 --- a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/federation/sync/SyncDummyUserFederationProviderFactory.java +++ b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/federation/sync/SyncDummyUserFederationProviderFactory.java @@ -42,12 +42,14 @@ import java.util.concurrent.TimeUnit; public class SyncDummyUserFederationProviderFactory extends DummyUserFederationProviderFactory { // Used during SyncFederationTest - public static volatile CountDownLatch latch1 = new CountDownLatch(1); - public static volatile CountDownLatch latch2 = new CountDownLatch(1); + public static volatile CountDownLatch latchStarted = new CountDownLatch(1); + public static volatile CountDownLatch latchWait = new CountDownLatch(1); + public static volatile CountDownLatch latchFinished = new CountDownLatch(1); public static void restartLatches() { - latch1 = new CountDownLatch(1); - latch2 = new CountDownLatch(1); + latchStarted = new CountDownLatch(1); + latchWait = new CountDownLatch(1); + latchFinished = new CountDownLatch(1); } @@ -77,6 +79,12 @@ public class SyncDummyUserFederationProviderFactory extends DummyUserFederationP @Override public SynchronizationResult syncSince(Date lastSync, KeycloakSessionFactory sessionFactory, String realmId, UserStorageProviderModel model) { + if (latchStarted.getCount() <= 0) { + logger.info("Already executed, returning"); + return SynchronizationResult.empty(); + } + // we are starting => allow the test to continue + latchStarted.countDown(); KeycloakModelUtils.runJobInTransaction(sessionFactory, new KeycloakSessionTask() { @@ -104,7 +112,8 @@ public class SyncDummyUserFederationProviderFactory extends DummyUserFederationP try { - latch1.await(waitTime * 1000, TimeUnit.MILLISECONDS); + // await the test to finish + latchWait.await(waitTime * 1000, TimeUnit.MILLISECONDS); } catch (InterruptedException ie) { Thread.currentThread().interrupt(); throw new RuntimeException("Interrupted!", ie); @@ -115,10 +124,10 @@ public class SyncDummyUserFederationProviderFactory extends DummyUserFederationP }); - // countDown, so the SyncFederationTest can continue - latch2.countDown(); + // countDown, so the SyncFederationTest can finish + latchFinished.countDown(); - return new SynchronizationResult(); + return SynchronizationResult.empty(); } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/sync/SyncFederationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/sync/SyncFederationTest.java index 02be689ad6..1d221a19f1 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/sync/SyncFederationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/sync/SyncFederationTest.java @@ -279,7 +279,7 @@ public class SyncFederationTest extends AbstractAuthTest { model.setFullSyncPeriod(-1); model.setChangedSyncPeriod(1); model.setLastSync(0); - model.getConfig().putSingle(SyncDummyUserFederationProviderFactory.WAIT_TIME, "2000"); + model.getConfig().putSingle(SyncDummyUserFederationProviderFactory.WAIT_TIME, "20"); ComponentModel dummyModel = new UserStorageProviderModel(appRealm.addComponentModel(model)); }); @@ -295,18 +295,30 @@ public class SyncFederationTest extends AbstractAuthTest { usersSyncManager.bootstrapPeriodic(sessionFactory, session.getProvider(TimerProvider.class)); // Wait and then trigger sync manually. Assert it will be ignored - sleep(1800); - SynchronizationResult syncResult = usersSyncManager.syncChangedUsers(sessionFactory, appRealm.getId(), dummyModel); - Assert.assertTrue(syncResult.isIgnored()); - - // Cancel timer - usersSyncManager.notifyToRefreshPeriodicSync(session, appRealm, dummyModel, true); - - // Signal to factory to finish waiting - SyncDummyUserFederationProviderFactory.latch1.countDown(); - try { - SyncDummyUserFederationProviderFactory.latch2.await(20000, TimeUnit.MILLISECONDS); + SyncDummyUserFederationProviderFactory.latchStarted.await(20000, TimeUnit.MILLISECONDS); + SynchronizationResult syncResult = usersSyncManager.syncChangedUsers(sessionFactory, appRealm.getId(), dummyModel); + Assert.assertTrue(syncResult.isIgnored()); + + // Cancel timer + usersSyncManager.notifyToRefreshPeriodicSync(session, appRealm, dummyModel, true); + + // Signal to factory to finish waiting + SyncDummyUserFederationProviderFactory.latchWait.countDown(); + + // wait the task to be finished + SyncDummyUserFederationProviderFactory.latchFinished.await(20000, TimeUnit.MILLISECONDS); + + // This sync is here just to ensure that we have lock (doublecheck that periodic sync, which was possibly triggered before canceling timer is finished too) + while (true) { + SynchronizationResult result = usersSyncManager.syncChangedUsers(session.getKeycloakSessionFactory(), appRealm.getId(), dummyModel); + if (result.isIgnored()) { + log.infof("Still waiting for lock before periodic sync is finished: %s", result.toString()); + sleep(1000); + } else { + break; + } + } } catch (Exception e) { throw new RuntimeException(e); }