Skip to content

Commit 2c190cc

Browse files
committed
#16612 fix flaky test: wait until the grid node is fully stopped
At least on my machine, stopping the node takes some time, and any checks right after `node.stop(sessionId)` often can fail.
1 parent 79ff056 commit 2c190cc

File tree

2 files changed

+54
-30
lines changed

2 files changed

+54
-30
lines changed

java/src/org/openqa/selenium/grid/data/NodeStatus.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,4 +248,10 @@ public Map<String, Object> toJson() {
248248

249249
return unmodifiableMap(toReturn);
250250
}
251+
252+
@Override
253+
public String toString() {
254+
return String.format(
255+
"NodeStatus(availability: %s, slots:%s, uri:%s)", availability, slots.size(), externalUri);
256+
}
251257
}

java/test/org/openqa/selenium/grid/node/local/LocalNodeTest.java

Lines changed: 48 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
package org.openqa.selenium.grid.node.local;
1919

20+
import static java.lang.System.currentTimeMillis;
2021
import static java.util.concurrent.TimeUnit.SECONDS;
2122
import static org.assertj.core.api.Assertions.assertThat;
2223
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
@@ -27,6 +28,7 @@
2728
import com.google.common.collect.ImmutableSet;
2829
import java.net.URI;
2930
import java.net.URISyntaxException;
31+
import java.time.Duration;
3032
import java.time.Instant;
3133
import java.util.ArrayList;
3234
import java.util.List;
@@ -117,22 +119,31 @@ void isOwnerOfAnActiveSession() {
117119

118120
@Test
119121
void canStopASession() {
120-
node.stop(session.getId());
122+
SessionId sessionId = session.getId();
123+
assertThat(node.getSession(sessionId)).isNotNull();
124+
125+
node.stop(sessionId);
126+
127+
waitUntilNodeStopped(sessionId);
121128
assertThatExceptionOfType(NoSuchSessionException.class)
122-
.isThrownBy(() -> node.getSession(session.getId()));
129+
.isThrownBy(() -> node.getSession(sessionId));
123130
}
124131

125132
@Test
126133
void isNotOwnerOfAStoppedSession() {
127134
node.stop(session.getId());
135+
136+
waitUntilNodeStopped(session.getId());
128137
assertThat(node.isSessionOwner(session.getId())).isFalse();
129138
}
130139

131140
@Test
132141
void cannotAcceptNewSessionsWhileDraining() {
133142
node.drain();
134143
assertThat(node.isDraining()).isTrue();
144+
135145
node.stop(session.getId()); // stop the default session
146+
waitUntilNodeStopped(session.getId());
136147

137148
Capabilities stereotype = new ImmutableCapabilities("cheese", "brie");
138149
Either<WebDriverException, CreateSessionResponse> sessionResponse =
@@ -155,41 +166,25 @@ void cannotCreateNewSessionsOnMaxSessionCount() {
155166

156167
@Test
157168
void canReturnStatusInfo() {
158-
NodeStatus status = node.getStatus();
159-
assertThat(
160-
status.getSlots().stream()
161-
.map(Slot::getSession)
162-
.filter(Objects::nonNull)
163-
.filter(s -> s.getId().equals(session.getId())))
164-
.isNotEmpty();
169+
SessionId sessionId = session.getId();
170+
assertThat(findSession(sessionId)).isNotEmpty();
165171

166-
node.stop(session.getId());
167-
status = node.getStatus();
168-
assertThat(
169-
status.getSlots().stream()
170-
.map(Slot::getSession)
171-
.filter(Objects::nonNull)
172-
.filter(s -> s.getId().equals(session.getId())))
173-
.isEmpty();
172+
node.stop(sessionId);
173+
waitUntilNodeStopped(sessionId);
174+
175+
assertThat(findSession(sessionId)).isEmpty();
174176
}
175177

176178
@Test
177179
void nodeStatusInfoIsImmutable() {
180+
SessionId sessionId = session.getId();
178181
NodeStatus status = node.getStatus();
179-
assertThat(
180-
status.getSlots().stream()
181-
.map(Slot::getSession)
182-
.filter(Objects::nonNull)
183-
.filter(s -> s.getId().equals(session.getId())))
184-
.isNotEmpty();
182+
assertThat(findSession(status, sessionId)).isNotEmpty();
185183

186-
node.stop(session.getId());
187-
assertThat(
188-
status.getSlots().stream()
189-
.map(Slot::getSession)
190-
.filter(Objects::nonNull)
191-
.filter(s -> s.getId().equals(session.getId())))
192-
.isNotEmpty();
184+
node.stop(sessionId);
185+
waitUntilNodeStopped(sessionId);
186+
187+
assertThat(findSession(status, sessionId)).isNotEmpty();
193188
}
194189

195190
@Test
@@ -441,4 +436,27 @@ void extractsFileNameFromRequestUri() {
441436
assertThat(node.extractFileName("/session/1234/se/files/файл+with+tähtedega.png"))
442437
.isEqualTo("файл+with+tähtedega.png");
443438
}
439+
440+
private void waitUntilNodeStopped(SessionId sessionId) {
441+
long timeout = Duration.ofSeconds(5).toMillis();
442+
443+
for (long start = currentTimeMillis(); currentTimeMillis() - start < timeout; ) {
444+
if (findSession(sessionId).isEmpty()) {
445+
break;
446+
}
447+
}
448+
}
449+
450+
private Optional<Session> findSession(SessionId sessionId) {
451+
NodeStatus status = node.getStatus();
452+
return findSession(status, sessionId);
453+
}
454+
455+
private Optional<Session> findSession(NodeStatus status, SessionId sessionId) {
456+
return status.getSlots().stream()
457+
.map(Slot::getSession)
458+
.filter(Objects::nonNull)
459+
.filter(s -> s.getId().equals(sessionId))
460+
.findAny();
461+
}
444462
}

0 commit comments

Comments
 (0)