From b5f80499491ad3dd533916e19be44a7cef23753c Mon Sep 17 00:00:00 2001 From: Danny Faught Date: Fri, 29 Sep 2023 16:18:32 -0700 Subject: [PATCH 1/4] refactor: rename test method * Searching for "FAILED" in the CI output always found this method, whether it failed or not. --- .../java/org/cloudfoundry/identity/uaa/util/JsonUtilsTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metrics-data/src/test/java/org/cloudfoundry/identity/uaa/util/JsonUtilsTest.java b/metrics-data/src/test/java/org/cloudfoundry/identity/uaa/util/JsonUtilsTest.java index a88885b0016..c2ed6750de1 100644 --- a/metrics-data/src/test/java/org/cloudfoundry/identity/uaa/util/JsonUtilsTest.java +++ b/metrics-data/src/test/java/org/cloudfoundry/identity/uaa/util/JsonUtilsTest.java @@ -68,7 +68,7 @@ void testSerializeExcludingProperties() { } @Test - void testSerializeExcludingPropertiesInnerCallFailed() { + void testSerializeExcludingPropertiesInnerCallFails() { Map groupProperties = JsonUtils.readValue(jsonTestObjectString, new TypeReference>() {}); try { JsonUtils.serializeExcludingProperties(groupProperties, "limit.unkonwn"); From b74a606f88aae6cad07563abec1ccde3175750be Mon Sep 17 00:00:00 2001 From: Danny Faught Date: Fri, 29 Sep 2023 16:23:57 -0700 Subject: [PATCH 2/4] refactor: use JUnit 5 expectations * JUnit 5 has a better way to assert on an expected exception. * Cleaned up an unncessary type specification. * Fixed a typo. * Cleaned up an import. --- .../identity/uaa/util/JsonUtilsTest.java | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/metrics-data/src/test/java/org/cloudfoundry/identity/uaa/util/JsonUtilsTest.java b/metrics-data/src/test/java/org/cloudfoundry/identity/uaa/util/JsonUtilsTest.java index c2ed6750de1..c4ba1e837b3 100644 --- a/metrics-data/src/test/java/org/cloudfoundry/identity/uaa/util/JsonUtilsTest.java +++ b/metrics-data/src/test/java/org/cloudfoundry/identity/uaa/util/JsonUtilsTest.java @@ -10,8 +10,8 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.Assertions.fail; class JsonUtilsTest { private static final String jsonTestObjectString = "{\"pattern\":\"/pattern\",\"group\":\"group\",\"limit\":1000,\"category\":\"category\"}"; @@ -69,13 +69,10 @@ void testSerializeExcludingProperties() { @Test void testSerializeExcludingPropertiesInnerCallFails() { - Map groupProperties = JsonUtils.readValue(jsonTestObjectString, new TypeReference>() {}); - try { - JsonUtils.serializeExcludingProperties(groupProperties, "limit.unkonwn"); - fail("not expected"); - } catch (Exception e) { - assertTrue(e instanceof JsonUtils.JsonUtilException); - } + Map groupProperties = JsonUtils.readValue(jsonTestObjectString, new TypeReference<>() {}); + assertThrows(JsonUtils.JsonUtilException.class, () -> { + JsonUtils.serializeExcludingProperties(groupProperties, "limit.unknown"); + }); } @Test From 17cc3b5e38df17728572e6bfa10bd524a0c5ca44 Mon Sep 17 00:00:00 2001 From: Danny Faught Date: Fri, 29 Sep 2023 17:09:23 -0700 Subject: [PATCH 3/4] refactor: reword warning * Trying to make it easier to search for "FAILED" in the CI output to find failed tests. * Also tried to make the message easier to understand. --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 3f958cafd8a..ddca6f561b2 100644 --- a/build.gradle +++ b/build.gradle @@ -94,7 +94,7 @@ subprojects { def retryFailOnPassedAfterRetry = Boolean.parseBoolean( System.getProperty("failOnPassedAfterRetry", "false")); if (!retryFailOnPassedAfterRetry) - logger.warn("retry: Failed tests are to be retried and considered as passed if the retry passes.") + logger.warn("retry: Flaky tests will not make the test run fail because failOnPassedAfterRetry is false.") failOnPassedAfterRetry = retryFailOnPassedAfterRetry maxFailures = Integer.parseInt(System.getProperty("maxFailures", "3")) From 531db67033b19b9ec2bb1aa77c864cf899a0d4f4 Mon Sep 17 00:00:00 2001 From: Danny Faught Date: Fri, 29 Sep 2023 17:11:27 -0700 Subject: [PATCH 4/4] refactor: rename variable * The meaning of the name "retryFailOnPassedAfterRetry" was difficult to understand. It causes the full test run to fail when a flaky test is detected (both passes and fails on multiple runs). * Seems the indirection is required, first setting and checking a variable, then setting the "failOnPassedAfterRetry" property used by the retry plugin. --- build.gradle | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/build.gradle b/build.gradle index ddca6f561b2..6468f50dd3c 100644 --- a/build.gradle +++ b/build.gradle @@ -91,12 +91,13 @@ subprojects { jvmArgs += ["-Xmx1024m", "-XX:+StartAttachListener", "-XX:+HeapDumpOnOutOfMemoryError", "-XX:HeapDumpPath=/var/log/uaa-tests.hprof"] retry { - def retryFailOnPassedAfterRetry = Boolean.parseBoolean( + def failOnPassedAfterRetrySystemProperty = Boolean.parseBoolean( System.getProperty("failOnPassedAfterRetry", "false")); - if (!retryFailOnPassedAfterRetry) + if (!failOnPassedAfterRetrySystemProperty) logger.warn("retry: Flaky tests will not make the test run fail because failOnPassedAfterRetry is false.") - failOnPassedAfterRetry = retryFailOnPassedAfterRetry + // Configure the retry extension + failOnPassedAfterRetry = failOnPassedAfterRetrySystemProperty maxFailures = Integer.parseInt(System.getProperty("maxFailures", "3")) maxRetries = Integer.parseInt(System.getProperty("maxRetries", "1")) }