Skip to content

Tests that join the server thread in HttpTunnelServerTests may run indefinitely #29522

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
polarbear567 wants to merge 4 commits into from
Closed

Tests that join the server thread in HttpTunnelServerTests may run indefinitely #29522

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c9d1ec0
Use a timeout when joining the thread.
polarbear567 Jan 21, 2022
5b95c89
Upgrade to Solr 8.11.1
wilkinsona Jan 21, 2022
3c5c5d9
Merge branch '2.7.x'
wilkinsona Jan 21, 2022
1a349ce
Merge remote-tracking branch 'origin/leo/fix_29502' into leo/fix_29502
polarbear567 Jan 24, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions spring-boot-project/spring-boot-dependencies/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1277,10 +1277,7 @@ bom {
]
}
}
library("Solr", "8.8.2") {
prohibit("8.9.0") {
because "it depends on an artifact that is not on Maven Central"
}
library("Solr", "8.11.1") {
group("org.apache.solr") {
modules = [
"solr-analysis-extras",
Expand All @@ -1289,8 +1286,12 @@ bom {
"solr-core",
"solr-dataimporthandler",
"solr-dataimporthandler-extras",
"solr-gcs-repository",
"solr-jaegertracer-configurator",
"solr-langid",
"solr-ltr",
"solr-prometheus-exporter",
"solr-s3-repository",
"solr-solrj" {
exclude group: "org.slf4j", module: "jcl-over-slf4j"
},
Expand Down
20 changes: 10 additions & 10 deletions .../src/test/java/org/springframework/boot/devtools/tunnel/server/HttpTunnelServerTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ void initialRequestIsSentToServer() throws Exception {
this.servletRequest.setContent("hello".getBytes());
this.server.handle(this.request, this.response);
this.serverChannel.disconnect();
this.server.getServerThread().join();
this.server.getServerThread().join(2000);
Copy link
Member

@philwebb philwebb Jan 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should extract this to a constant in case we need to tweak the timings later?

Copy link
Member

@wilkinsona wilkinsona Jan 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1. I suspect we'll need longer than two seconds as well for it not to be flaky on CI.

this.serverChannel.verifyReceived("hello");
}

Expand All @@ -143,7 +143,7 @@ void initialRequestIsUsedForFirstServerResponse() throws Exception {
System.out.println("sending");
this.serverChannel.send("hello");
this.serverChannel.disconnect();
this.server.getServerThread().join();
this.server.getServerThread().join(2000);
assertThat(this.servletResponse.getContentAsString()).isEqualTo("hello");
this.serverChannel.verifyReceived("hello");
}
Expand All @@ -153,7 +153,7 @@ void initialRequestHasNoPayload() throws Exception {
givenServerConnectionOpenWillAnswerWithServerChannel();
this.server.handle(this.request, this.response);
this.serverChannel.disconnect();
this.server.getServerThread().join();
this.server.getServerThread().join(2000);
this.serverChannel.verifyReceived(NO_DATA);
}

Expand All @@ -176,7 +176,7 @@ void typicalRequestResponseTraffic() throws Exception {
this.serverChannel.send("=3");
h3.verifyReceived("=3", 3);
this.serverChannel.disconnect();
this.server.getServerThread().join();
this.server.getServerThread().join(2000);
}

@Test
Expand All @@ -185,7 +185,7 @@ void clientIsAwareOfServerClose() throws Exception {
MockHttpConnection h1 = new MockHttpConnection("1", 1);
this.server.handle(h1);
this.serverChannel.disconnect();
this.server.getServerThread().join();
this.server.getServerThread().join(2000);
assertThat(h1.getServletResponse().getStatus()).isEqualTo(410);
}

Expand All @@ -197,7 +197,7 @@ void clientCanCloseServer() throws Exception {
MockHttpConnection h2 = new MockHttpConnection("DISCONNECT", 1);
h2.getServletRequest().addHeader("Content-Type", "application/x-disconnect");
this.server.handle(h2);
this.server.getServerThread().join();
this.server.getServerThread().join(2000);
assertThat(h1.getServletResponse().getStatus()).isEqualTo(410);
assertThat(this.serverChannel.isOpen()).isFalse();
}
Expand All @@ -214,7 +214,7 @@ void neverMoreThanTwoHttpConnections() throws Exception {
h1.waitForResponse();
assertThat(h1.getServletResponse().getStatus()).isEqualTo(429);
this.serverChannel.disconnect();
this.server.getServerThread().join();
this.server.getServerThread().join(2000);
}

@Test
Expand All @@ -228,7 +228,7 @@ void requestReceivedOutOfOrder() throws Exception {
this.server.handle(h2);
this.serverChannel.verifyReceived("1+2+3");
this.serverChannel.disconnect();
this.server.getServerThread().join();
this.server.getServerThread().join(2000);
}

@Test
Expand All @@ -245,7 +245,7 @@ void httpConnectionsAreClosedAfterLongPollTimeout() throws Exception {
Awaitility.await().atMost(Duration.ofSeconds(30)).until(h2.getServletResponse()::getStatus,
(status) -> status == 204);
this.serverChannel.disconnect();
this.server.getServerThread().join();
this.server.getServerThread().join(2000);
}

@Test
Expand All @@ -256,7 +256,7 @@ void disconnectTimeout() throws Exception {
MockHttpConnection h1 = new MockHttpConnection();
this.server.handle(h1);
this.serverChannel.send("hello");
this.server.getServerThread().join();
this.server.getServerThread().join(2000);
assertThat(this.serverChannel.isOpen()).isFalse();
}

Expand Down