From d7138cdb6742eaf6ff8c58027a0f55c95a2699e7 Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Tue, 20 Aug 2024 17:57:02 -0600 Subject: [PATCH] Repair Flaky Tests The error between MockWebServer and OpenSAML still happens on occasion. This commit uses MockWebServer's default queue dispatcher to remove any customization that might be contributing to the flakiness. Issue gh-15395 --- ...AssertingPartyMetadataRepositoryTests.java | 42 +++++-------------- ...AssertingPartyMetadataRepositoryTests.java | 40 +++++------------- 2 files changed, 20 insertions(+), 62 deletions(-) diff --git a/saml2/saml2-service-provider/src/opensaml4Test/java/org/springframework/security/saml2/provider/service/registration/OpenSaml4AssertingPartyMetadataRepositoryTests.java b/saml2/saml2-service-provider/src/opensaml4Test/java/org/springframework/security/saml2/provider/service/registration/OpenSaml4AssertingPartyMetadataRepositoryTests.java index 5537768088..7e5b2d4cbd 100644 --- a/saml2/saml2-service-provider/src/opensaml4Test/java/org/springframework/security/saml2/provider/service/registration/OpenSaml4AssertingPartyMetadataRepositoryTests.java +++ b/saml2/saml2-service-provider/src/opensaml4Test/java/org/springframework/security/saml2/provider/service/registration/OpenSaml4AssertingPartyMetadataRepositoryTests.java @@ -90,7 +90,7 @@ public class OpenSaml4AssertingPartyMetadataRepositoryTests { @Test public void withMetadataUrlLocationWhenResolvableThenFindByEntityIdReturns() throws Exception { try (MockWebServer server = new MockWebServer()) { - server.setDispatcher(new AlwaysDispatch(this.metadata)); + enqueue(server, this.metadata, 3); AssertingPartyMetadataRepository parties = OpenSaml4AssertingPartyMetadataRepository .withTrustedMetadataLocation(server.url("/").toString()) .build(); @@ -107,7 +107,7 @@ public class OpenSaml4AssertingPartyMetadataRepositoryTests { @Test public void withMetadataUrlLocationnWhenResolvableThenIteratorReturns() throws Exception { try (MockWebServer server = new MockWebServer()) { - server.setDispatcher(new AlwaysDispatch(this.entitiesDescriptor)); + enqueue(server, this.entitiesDescriptor, 3); List parties = new ArrayList<>(); OpenSaml4AssertingPartyMetadataRepository.withTrustedMetadataLocation(server.url("/").toString()) .build() @@ -122,9 +122,6 @@ public class OpenSaml4AssertingPartyMetadataRepositoryTests { @Test public void withMetadataUrlLocationWhenUnresolvableThenThrowsSaml2Exception() throws Exception { try (MockWebServer server = new MockWebServer()) { - server.enqueue(new MockResponse().setBody(this.metadata) - .setResponseCode(200) - .setBodyDelay(1, TimeUnit.MILLISECONDS)); String url = server.url("/").toString(); server.shutdown(); assertThatExceptionOfType(Saml2Exception.class) @@ -135,7 +132,7 @@ public class OpenSaml4AssertingPartyMetadataRepositoryTests { @Test public void withMetadataUrlLocationWhenMalformedResponseThenSaml2Exception() throws Exception { try (MockWebServer server = new MockWebServer()) { - server.setDispatcher(new AlwaysDispatch("malformed")); + enqueue(server, "malformed", 3); String url = server.url("/").toString(); assertThatExceptionOfType(Saml2Exception.class) .isThrownBy(() -> OpenSaml4AssertingPartyMetadataRepository.withTrustedMetadataLocation(url).build()); @@ -218,8 +215,7 @@ public class OpenSaml4AssertingPartyMetadataRepositoryTests { Credential credential = TestOpenSamlObjects .getSigningCredential(TestSaml2X509Credentials.relyingPartyVerifyingCredential(), descriptor.getEntityID()); try (MockWebServer server = new MockWebServer()) { - server.start(); - server.setDispatcher(new AlwaysDispatch(serialized)); + enqueue(server, serialized, 3); AssertingPartyMetadataRepository parties = OpenSaml4AssertingPartyMetadataRepository .withTrustedMetadataLocation(server.url("/").toString()) .verificationCredentials((c) -> c.add(credential)) @@ -238,8 +234,7 @@ public class OpenSaml4AssertingPartyMetadataRepositoryTests { Credential credential = TestOpenSamlObjects .getSigningCredential(TestSaml2X509Credentials.relyingPartyVerifyingCredential(), descriptor.getEntityID()); try (MockWebServer server = new MockWebServer()) { - server.start(); - server.setDispatcher(new AlwaysDispatch(serialized)); + enqueue(server, serialized, 3); assertThatExceptionOfType(Saml2Exception.class).isThrownBy(() -> OpenSaml4AssertingPartyMetadataRepository .withTrustedMetadataLocation(server.url("/").toString()) .verificationCredentials((c) -> c.add(credential)) @@ -255,8 +250,7 @@ public class OpenSaml4AssertingPartyMetadataRepositoryTests { descriptor.getEntityID()); String serialized = serialize(descriptor); try (MockWebServer server = new MockWebServer()) { - server.start(); - server.setDispatcher(new AlwaysDispatch(serialized)); + enqueue(server, serialized, 3); AssertingPartyMetadataRepository parties = OpenSaml4AssertingPartyMetadataRepository .withTrustedMetadataLocation(server.url("/").toString()) .build(); @@ -336,8 +330,7 @@ public class OpenSaml4AssertingPartyMetadataRepositoryTests { Credential credential = TestOpenSamlObjects .getSigningCredential(TestSaml2X509Credentials.relyingPartyVerifyingCredential(), descriptor.getEntityID()); try (MockWebServer server = new MockWebServer()) { - server.start(); - server.setDispatcher(new AlwaysDispatch(serialized)); + enqueue(server, serialized, 3); AssertingPartyMetadataRepository parties = OpenSaml4AssertingPartyMetadataRepository .withMetadataLocation(server.url("/").toString()) .verificationCredentials((c) -> c.add(credential)) @@ -357,25 +350,10 @@ public class OpenSaml4AssertingPartyMetadataRepositoryTests { } } - private static final class AlwaysDispatch extends Dispatcher { - - private final MockResponse response; - - private AlwaysDispatch(String body) { - this.response = new MockResponse().setBody(body) - .setResponseCode(200) - .setBodyDelay(1, TimeUnit.MILLISECONDS); + private static void enqueue(MockWebServer web, String body, int times) { + for (int i = 0; i < times; i++) { + web.enqueue(new MockResponse().setBody(body).setResponseCode(200)); } - - private AlwaysDispatch(MockResponse response) { - this.response = response; - } - - @Override - public MockResponse dispatch(RecordedRequest recordedRequest) throws InterruptedException { - return this.response; - } - } } diff --git a/saml2/saml2-service-provider/src/opensaml5Test/java/org/springframework/security/saml2/provider/service/registration/OpenSaml5AssertingPartyMetadataRepositoryTests.java b/saml2/saml2-service-provider/src/opensaml5Test/java/org/springframework/security/saml2/provider/service/registration/OpenSaml5AssertingPartyMetadataRepositoryTests.java index 6184ed1db3..f13af3ceb7 100644 --- a/saml2/saml2-service-provider/src/opensaml5Test/java/org/springframework/security/saml2/provider/service/registration/OpenSaml5AssertingPartyMetadataRepositoryTests.java +++ b/saml2/saml2-service-provider/src/opensaml5Test/java/org/springframework/security/saml2/provider/service/registration/OpenSaml5AssertingPartyMetadataRepositoryTests.java @@ -90,7 +90,7 @@ public class OpenSaml5AssertingPartyMetadataRepositoryTests { @Test public void withMetadataUrlLocationWhenResolvableThenFindByEntityIdReturns() throws Exception { try (MockWebServer server = new MockWebServer()) { - server.setDispatcher(new AlwaysDispatch(this.metadata)); + enqueue(server, this.metadata, 3); AssertingPartyMetadataRepository parties = OpenSaml5AssertingPartyMetadataRepository .withTrustedMetadataLocation(server.url("/").toString()) .build(); @@ -107,7 +107,7 @@ public class OpenSaml5AssertingPartyMetadataRepositoryTests { @Test public void withMetadataUrlLocationnWhenResolvableThenIteratorReturns() throws Exception { try (MockWebServer server = new MockWebServer()) { - server.setDispatcher(new AlwaysDispatch(this.entitiesDescriptor)); + enqueue(server, this.entitiesDescriptor, 3); List parties = new ArrayList<>(); OpenSaml5AssertingPartyMetadataRepository.withTrustedMetadataLocation(server.url("/").toString()) .build() @@ -122,7 +122,6 @@ public class OpenSaml5AssertingPartyMetadataRepositoryTests { @Test public void withMetadataUrlLocationWhenUnresolvableThenThrowsSaml2Exception() throws Exception { try (MockWebServer server = new MockWebServer()) { - server.enqueue(new MockResponse().setBody(this.metadata).setResponseCode(200)); String url = server.url("/").toString(); server.shutdown(); assertThatExceptionOfType(Saml2Exception.class) @@ -133,7 +132,7 @@ public class OpenSaml5AssertingPartyMetadataRepositoryTests { @Test public void withMetadataUrlLocationWhenMalformedResponseThenSaml2Exception() throws Exception { try (MockWebServer server = new MockWebServer()) { - server.setDispatcher(new AlwaysDispatch("malformed")); + enqueue(server, "malformed", 3); String url = server.url("/").toString(); assertThatExceptionOfType(Saml2Exception.class) .isThrownBy(() -> OpenSaml5AssertingPartyMetadataRepository.withTrustedMetadataLocation(url).build()); @@ -216,8 +215,7 @@ public class OpenSaml5AssertingPartyMetadataRepositoryTests { Credential credential = TestOpenSamlObjects .getSigningCredential(TestSaml2X509Credentials.relyingPartyVerifyingCredential(), descriptor.getEntityID()); try (MockWebServer server = new MockWebServer()) { - server.start(); - server.setDispatcher(new AlwaysDispatch(serialized)); + enqueue(server, serialized, 3); AssertingPartyMetadataRepository parties = OpenSaml5AssertingPartyMetadataRepository .withTrustedMetadataLocation(server.url("/").toString()) .verificationCredentials((c) -> c.add(credential)) @@ -236,8 +234,7 @@ public class OpenSaml5AssertingPartyMetadataRepositoryTests { Credential credential = TestOpenSamlObjects .getSigningCredential(TestSaml2X509Credentials.relyingPartyVerifyingCredential(), descriptor.getEntityID()); try (MockWebServer server = new MockWebServer()) { - server.start(); - server.setDispatcher(new AlwaysDispatch(serialized)); + enqueue(server, serialized, 3); assertThatExceptionOfType(Saml2Exception.class).isThrownBy(() -> OpenSaml5AssertingPartyMetadataRepository .withTrustedMetadataLocation(server.url("/").toString()) .verificationCredentials((c) -> c.add(credential)) @@ -253,8 +250,7 @@ public class OpenSaml5AssertingPartyMetadataRepositoryTests { descriptor.getEntityID()); String serialized = serialize(descriptor); try (MockWebServer server = new MockWebServer()) { - server.start(); - server.setDispatcher(new AlwaysDispatch(serialized)); + enqueue(server, serialized, 3); AssertingPartyMetadataRepository parties = OpenSaml5AssertingPartyMetadataRepository .withTrustedMetadataLocation(server.url("/").toString()) .build(); @@ -334,8 +330,7 @@ public class OpenSaml5AssertingPartyMetadataRepositoryTests { Credential credential = TestOpenSamlObjects .getSigningCredential(TestSaml2X509Credentials.relyingPartyVerifyingCredential(), descriptor.getEntityID()); try (MockWebServer server = new MockWebServer()) { - server.start(); - server.setDispatcher(new AlwaysDispatch(serialized)); + enqueue(server, serialized, 3); AssertingPartyMetadataRepository parties = OpenSaml5AssertingPartyMetadataRepository .withMetadataLocation(server.url("/").toString()) .verificationCredentials((c) -> c.add(credential)) @@ -355,25 +350,10 @@ public class OpenSaml5AssertingPartyMetadataRepositoryTests { } } - private static final class AlwaysDispatch extends Dispatcher { - - private final MockResponse response; - - private AlwaysDispatch(String body) { - this.response = new MockResponse().setBody(body) - .setResponseCode(200) - .setBodyDelay(1, TimeUnit.MILLISECONDS); + private static void enqueue(MockWebServer web, String body, int times) { + for (int i = 0; i < times; i++) { + web.enqueue(new MockResponse().setBody(body).setResponseCode(200)); } - - private AlwaysDispatch(MockResponse response) { - this.response = response; - } - - @Override - public MockResponse dispatch(RecordedRequest recordedRequest) throws InterruptedException { - return this.response; - } - } }