From 37e592c16659bd0ea7d265c6f3cb969ac81e5524 Mon Sep 17 00:00:00 2001 From: Puja Jagani Date: Tue, 10 Nov 2020 19:49:53 +0530 Subject: [PATCH] [grid] Fix flaky Distributor and GraphqlHandlerTest. Add queuer config to DistributedCdpTest (#8859) * Add queuer config to DistributedCdpTest set up. * Add wait to DistributorTest and GraphqlHandlerTest to allow node to be added before creating session. --- .../grid/distributor/DistributorTest.java | 67 +++++++++++-------- .../openqa/selenium/grid/graphql/BUILD.bazel | 1 + .../grid/graphql/GraphqlHandlerTest.java | 14 ++++ .../grid/router/DistributedCdpTest.java | 13 +++- 4 files changed, 67 insertions(+), 28 deletions(-) diff --git a/java/server/test/org/openqa/selenium/grid/distributor/DistributorTest.java b/java/server/test/org/openqa/selenium/grid/distributor/DistributorTest.java index 1ea8ae2c8f8e2..6f67ffae2f6a7 100644 --- a/java/server/test/org/openqa/selenium/grid/distributor/DistributorTest.java +++ b/java/server/test/org/openqa/selenium/grid/distributor/DistributorTest.java @@ -104,6 +104,7 @@ public class DistributorTest { private Capabilities stereotype; private Capabilities caps; private final Secret registrationSecret = new Secret("hellim"); + private final Wait wait = new FluentWait<>(new Object()).withTimeout(Duration.ofSeconds(5)); @Before public void setUp() { @@ -309,6 +310,8 @@ public void testDrainedNodeShutsDownOnceEmpty() throws URISyntaxException, Inter queuer, registrationSecret); distributor.add(node); + wait.until(obj -> distributor.getStatus().hasCapacity()); + distributor.drain(node.getId()); latch.await(5, TimeUnit.SECONDS); @@ -352,6 +355,8 @@ public void drainedNodeDoesNotShutDownIfNotEmpty() registrationSecret); distributor.add(node); + wait.until(obj -> distributor.getStatus().hasCapacity()); + NewSessionPayload payload = NewSessionPayload.create(caps); distributor.newSession(createRequest(payload)); @@ -394,6 +399,8 @@ public void drainedNodeShutsDownAfterSessionsFinish() registrationSecret); distributor.add(node); + wait.until(obj -> distributor.getStatus().hasCapacity()); + NewSessionPayload payload = NewSessionPayload.create(caps); CreateSessionResponse firstResponse = distributor.newSession(createRequest(payload)); CreateSessionResponse secondResponse = distributor.newSession(createRequest(payload)); @@ -504,6 +511,8 @@ public void theMostLightlyLoadedNodeIsSelectedFirst() { .add(lightest) .add(massive); + wait.until(obj -> distributor.getStatus().hasCapacity()); + try (NewSessionPayload payload = NewSessionPayload.create(caps)) { Session session = distributor.newSession(createRequest(payload)).getSession(); @@ -633,7 +642,9 @@ public void shouldIncludeHostsThatAreUpInHostList() { } @Test - public void shouldNotScheduleAJobIfAllSlotsAreBeingUsed() { + public void shouldNotScheduleAJobIfAllSlotsAreBeingUsed() throws URISyntaxException { + URI nodeUri = new URI("http://example:5678"); + URI routableUri = new URI("http://localhost:1234"); SessionMap sessions = new LocalSessionMap(tracer, bus); LocalNewSessionQueue localNewSessionQueue = new LocalNewSessionQueue( tracer, @@ -642,19 +653,20 @@ public void shouldNotScheduleAJobIfAllSlotsAreBeingUsed() { Duration.ofSeconds(2)); LocalNewSessionQueuer queuer = new LocalNewSessionQueuer(tracer, bus, localNewSessionQueue); - CombinedHandler handler = new CombinedHandler(); + LocalNode node = LocalNode.builder(tracer, bus, routableUri, routableUri, registrationSecret) + .add(caps, new TestSessionFactory((id, c) -> new Session( + id, nodeUri, stereotype, c, Instant.now()))) + .build(); Distributor distributor = new LocalDistributor( tracer, bus, - new PassthroughHttpClient.Factory(handler), + new PassthroughHttpClient.Factory(node), sessions, queuer, registrationSecret); - handler.addHandler(distributor); - Node node = createNode(caps, 1, 0); - handler.addHandler(node); distributor.add(node); + wait.until(obj -> distributor.getStatus().hasCapacity()); // Use up the one slot available try (NewSessionPayload payload = NewSessionPayload.create(caps)) { @@ -669,7 +681,9 @@ public void shouldNotScheduleAJobIfAllSlotsAreBeingUsed() { } @Test - public void shouldReleaseSlotOnceSessionEnds() { + public void shouldReleaseSlotOnceSessionEnds() throws URISyntaxException { + URI nodeUri = new URI("http://example:5678"); + URI routableUri = new URI("http://localhost:1234"); SessionMap sessions = new LocalSessionMap(tracer, bus); LocalNewSessionQueue localNewSessionQueue = new LocalNewSessionQueue( tracer, @@ -678,20 +692,22 @@ public void shouldReleaseSlotOnceSessionEnds() { Duration.ofSeconds(2)); LocalNewSessionQueuer queuer = new LocalNewSessionQueuer(tracer, bus, localNewSessionQueue); - CombinedHandler handler = new CombinedHandler(); + LocalNode node = LocalNode.builder(tracer, bus, routableUri, routableUri, registrationSecret) + .add(caps, new TestSessionFactory((id, c) -> new Session( + id, nodeUri, stereotype, c, Instant.now()))) + .build(); + Distributor distributor = new LocalDistributor( tracer, bus, - new PassthroughHttpClient.Factory(handler), + new PassthroughHttpClient.Factory(node), sessions, queuer, registrationSecret); - handler.addHandler(distributor); - - Node node = createNode(caps, 1, 0); - handler.addHandler(node); distributor.add(node); + wait.until(obj -> distributor.getStatus().hasCapacity()); + // Use up the one slot available Session session; try (NewSessionPayload payload = NewSessionPayload.create(caps)) { @@ -702,9 +718,7 @@ public void shouldReleaseSlotOnceSessionEnds() { sessions.get(session.getId()); node.stop(session.getId()); - // Now wait for the session map to say the session is gone. - Wait wait = new FluentWait<>(new Object()).withTimeout(Duration.ofSeconds(2)); wait.until(obj -> { try { sessions.get(session.getId()); @@ -756,8 +770,9 @@ public void shouldNotStartASessionIfTheCapabilitiesAreNotSupported() { } @Test - public void attemptingToStartASessionWhichFailsMarksAsTheSlotAsAvailable() { - CombinedHandler handler = new CombinedHandler(); + public void attemptingToStartASessionWhichFailsMarksAsTheSlotAsAvailable() throws URISyntaxException { + URI nodeUri = new URI("http://example:5678"); + URI routableUri = new URI("http://localhost:1234"); SessionMap sessions = new LocalSessionMap(tracer, bus); LocalNewSessionQueue localNewSessionQueue = new LocalNewSessionQueue( @@ -766,26 +781,24 @@ public void attemptingToStartASessionWhichFailsMarksAsTheSlotAsAvailable() { Duration.ofSeconds(2), Duration.ofSeconds(2)); LocalNewSessionQueuer queuer = new LocalNewSessionQueuer(tracer, bus, localNewSessionQueue); - handler.addHandler(sessions); - URI uri = createUri(); - Node node = LocalNode.builder(tracer, bus, uri, uri, registrationSecret) - .add(caps, new TestSessionFactory((id, caps) -> { - throw new SessionNotCreatedException("OMG"); - })) - .build(); - handler.addHandler(node); + LocalNode node = LocalNode.builder(tracer, bus, routableUri, routableUri, registrationSecret) + .add(caps, new TestSessionFactory((id, caps) -> { + throw new SessionNotCreatedException("OMG"); + })) + .build(); Distributor distributor = new LocalDistributor( tracer, bus, - new PassthroughHttpClient.Factory(handler), + new PassthroughHttpClient.Factory(node), sessions, queuer, registrationSecret); - handler.addHandler(distributor); distributor.add(node); + wait.until(obj -> distributor.getStatus().hasCapacity()); + try (NewSessionPayload payload = NewSessionPayload.create(caps)) { assertThatExceptionOfType(SessionNotCreatedException.class) .isThrownBy(() -> distributor.newSession(createRequest(payload))); diff --git a/java/server/test/org/openqa/selenium/grid/graphql/BUILD.bazel b/java/server/test/org/openqa/selenium/grid/graphql/BUILD.bazel index 36caeb6b16752..11f9e004155fb 100644 --- a/java/server/test/org/openqa/selenium/grid/graphql/BUILD.bazel +++ b/java/server/test/org/openqa/selenium/grid/graphql/BUILD.bazel @@ -14,6 +14,7 @@ java_test_suite( deps = [ "//java/client/src/org/openqa/selenium/json", "//java/client/src/org/openqa/selenium/remote", + "//java/client/src/org/openqa/selenium/support", "//java/client/test/org/openqa/selenium/remote/tracing:tracing-support", "//java/server/src/org/openqa/selenium/events", "//java/server/src/org/openqa/selenium/events/local", diff --git a/java/server/test/org/openqa/selenium/grid/graphql/GraphqlHandlerTest.java b/java/server/test/org/openqa/selenium/grid/graphql/GraphqlHandlerTest.java index e7cbee76751d7..4259bed8dd496 100644 --- a/java/server/test/org/openqa/selenium/grid/graphql/GraphqlHandlerTest.java +++ b/java/server/test/org/openqa/selenium/grid/graphql/GraphqlHandlerTest.java @@ -48,6 +48,8 @@ import org.openqa.selenium.remote.http.HttpResponse; import org.openqa.selenium.remote.tracing.DefaultTestTracer; import org.openqa.selenium.remote.tracing.Tracer; +import org.openqa.selenium.support.ui.FluentWait; +import org.openqa.selenium.support.ui.Wait; import java.io.IOException; import java.io.UncheckedIOException; @@ -77,6 +79,7 @@ public class GraphqlHandlerTest { private ImmutableCapabilities caps; private ImmutableCapabilities stereotype; private NewSessionPayload payload; + private final Wait wait = new FluentWait<>(new Object()).withTimeout(Duration.ofSeconds(5)); public GraphqlHandlerTest() throws URISyntaxException { } @@ -146,6 +149,7 @@ public boolean test(Capabilities capabilities) { }) .build(); distributor.add(node); + wait.until(obj -> distributor.getStatus().hasCapacity()); GraphqlHandler handler = new GraphqlHandler(distributor, publicUri); Map topLevel = executeQuery(handler, "{ grid { nodes { uri } } }"); @@ -169,6 +173,8 @@ public void shouldBeAbleToGetSessionInfo() throws URISyntaxException { Instant.now()))).build(); distributor.add(node); + wait.until(obj -> distributor.getStatus().hasCapacity()); + Session session = distributor.newSession(createRequest(payload)).getSession(); assertThat(session).isNotNull(); @@ -215,6 +221,8 @@ public void shouldBeAbleToGetNodeInfoForSession() throws URISyntaxException { Instant.now()))).build(); distributor.add(node); + wait.until(obj -> distributor.getStatus().hasCapacity()); + Session session = distributor.newSession(createRequest(payload)).getSession(); assertThat(session).isNotNull(); @@ -260,6 +268,8 @@ public void shouldBeAbleToGetSlotInfoForSession() throws URISyntaxException { Instant.now()))).build(); distributor.add(node); + wait.until(obj -> distributor.getStatus().hasCapacity()); + Session session = distributor.newSession(createRequest(payload)).getSession(); assertThat(session).isNotNull(); @@ -311,6 +321,8 @@ public void shouldBeAbleToGetSessionDuration() throws URISyntaxException { Instant.now()))).build(); distributor.add(node); + wait.until(obj -> distributor.getStatus().hasCapacity()); + Session session = distributor.newSession(createRequest(payload)).getSession(); assertThat(session).isNotNull(); @@ -341,6 +353,7 @@ public void shouldThrowExceptionWhenSessionNotFound() throws URISyntaxException Instant.now()))).build(); distributor.add(node); + wait.until(obj -> distributor.getStatus().hasCapacity()); String randomSessionId = UUID.randomUUID().toString(); String query = "{ session (id: \"" + randomSessionId + "\") { sessionDurationMillis } }"; @@ -376,6 +389,7 @@ public void shouldThrowExceptionWhenSessionIsEmpty() throws URISyntaxException { Instant.now()))).build(); distributor.add(node); + wait.until(obj -> distributor.getStatus().hasCapacity()); String query = "{ session (id: \"\") { sessionDurationMillis } }"; diff --git a/java/server/test/org/openqa/selenium/grid/router/DistributedCdpTest.java b/java/server/test/org/openqa/selenium/grid/router/DistributedCdpTest.java index d389b56a3092c..dedde47710493 100644 --- a/java/server/test/org/openqa/selenium/grid/router/DistributedCdpTest.java +++ b/java/server/test/org/openqa/selenium/grid/router/DistributedCdpTest.java @@ -32,6 +32,7 @@ import org.openqa.selenium.grid.server.BaseServerOptions; import org.openqa.selenium.grid.server.Server; import org.openqa.selenium.grid.sessionmap.httpd.SessionMapServer; +import org.openqa.selenium.grid.sessionqueue.httpd.NewSessionQueuerServer; import org.openqa.selenium.grid.web.Values; import org.openqa.selenium.net.PortProber; import org.openqa.selenium.netty.server.NettyServer; @@ -84,11 +85,21 @@ public void ensureBasicFunctionality() throws MalformedURLException, Interrupted mergeArgs(eventBusFlags, "--bind-bus", "false", "--port", "" + sessionsPort)).run(); waitUntilUp(sessionsPort); + int queuerPort = PortProber.findFreePort(); + new NewSessionQueuerServer().configure( + System.out, + System.err, + mergeArgs(eventBusFlags, "--bind-bus", "false", "--port", "" + queuerPort)).run(); + waitUntilUp(sessionsPort); + int distributorPort = PortProber.findFreePort(); new DistributorServer().configure( System.out, System.err, - mergeArgs(eventBusFlags, "--bind-bus", "false", "--port", "" + distributorPort, "-s", "http://localhost:" + sessionsPort)).run(); + mergeArgs(eventBusFlags, + "--bind-bus", "false", "--port", "" + distributorPort, + "-s", "http://localhost:" + sessionsPort, + "--sessionqueuer", "http://localhost:" + queuerPort)).run(); waitUntilUp(distributorPort); int routerPort = PortProber.findFreePort();