Skip to content

Commit 2e6d38f

Browse files
committed
Revert back to before(:all) for adapter_lint
Using let-style RSpec and lazily creating our servers before every test is good from a test purity point of view but increases greatly the risk of port collisions when we attempt to find a free port to run our adapter in adapter_lint (bearing in mind that Socket#close only releases the Ruby resources, and the socket persists in a TIME_WAIT state for an indeterminate period of time at OS level). Revert to before(:all) and create only one server that will be used for the duration of all tests in adapter_lint. We still need to sleep to ensure our 'free' ports are available, but we reduce the risk of servers being unavailable. This also has the happy side effect of making the tests somewhat faster.
1 parent 81c4eea commit 2e6d38f

File tree

1 file changed

+42
-38
lines changed

1 file changed

+42
-38
lines changed

lib/webmachine/spec/adapter_lint.rb

Lines changed: 42 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,68 +1,72 @@
11
require "webmachine/spec/test_resource"
22
require "net/http"
33

4+
ADDRESS = "127.0.0.1"
5+
46
shared_examples_for :adapter_lint do
5-
attr_accessor :client
6-
7-
let(:address) { "127.0.0.1" }
8-
let(:port) do
9-
s = TCPServer.new(address, 0)
10-
p = s.addr[1]
11-
s.close # This does not close the socket at OS level, just frees from Ruby.
12-
# The socket will be in TIME_WAIT http://www.ssfnet.org/Exchange/tcp/tcpTutorialNotes.html
13-
# "The main thing to recognize about connection teardown is that a connection in
14-
# the TIME_WAIT state cannot move to the CLOSED state until it has waited for two times
15-
# the maximum amount of time an IP datagram might live in the Inter net."
16-
17-
sleep(0.005) # This is just about the best we can do. Any more slows the tests,
18-
# any less and we get intermittent silent port collisions
19-
p
20-
end
21-
22-
let(:application) do
23-
application = Webmachine::Application.new
24-
application.dispatcher.add_route ["test"], Test::Resource
25-
26-
application.configure do |c|
27-
c.ip = address
28-
c.port = port
7+
attr_reader :client
8+
9+
class TestApplicationNotResponsive < Timeout::Error; end
10+
11+
def find_free_port
12+
temp_server = TCPServer.new(ADDRESS, 0)
13+
port = temp_server.addr[1]
14+
temp_server.close # only frees Ruby resource, socket is in TIME_WAIT at OS level
15+
# so we can't have our adapter use it too quickly
16+
17+
sleep(0.1) # 'Wait' for temp_server to *really* close, not just TIME_WAIT
18+
port
19+
end
20+
21+
def create_test_application(port)
22+
Webmachine::Application.new.tap do |application|
23+
application.dispatcher.add_route ["test"], Test::Resource
24+
25+
application.configure do |c|
26+
c.ip = ADDRESS
27+
c.port = port
28+
end
2929
end
30+
end
3031

31-
application
32+
def run_application(adapter_class, application)
33+
adapter = adapter_class.new(application)
34+
Thread.abort_on_exception = true
35+
Thread.new { adapter.run }
3236
end
3337

34-
let(:client) do
35-
client = Net::HTTP.new(application.configuration.ip, port)
36-
# Wait until the server is responsive
37-
Timeout.timeout(5) do
38+
def wait_until_server_responds_to(client)
39+
Timeout.timeout(5, TestApplicationNotResponsive) do
3840
begin
3941
client.start
4042
rescue Errno::ECONNREFUSED
4143
sleep(0.01)
4244
retry
4345
end
4446
end
45-
client
4647
end
4748

48-
before do
49-
@adapter = described_class.new(application)
49+
before(:all) do
50+
@port = find_free_port
51+
application = create_test_application(@port)
5052

51-
Thread.abort_on_exception = true
52-
@server_thread = Thread.new { @adapter.run }
53-
sleep(0.01)
53+
adapter_class = described_class
54+
@server_thread = run_application(adapter_class, application)
55+
56+
@client = Net::HTTP.new(application.configuration.ip, @port)
57+
wait_until_server_responds_to(client)
5458
end
5559

56-
after do
57-
client.finish
60+
after(:all) do
61+
@client.finish
5862
@server_thread.kill
5963
end
6064

6165
it "provides the request URI" do
6266
request = Net::HTTP::Get.new("/test")
6367
request["Accept"] = "test/response.request_uri"
6468
response = client.request(request)
65-
expect(response.body).to eq("http://#{address}:#{port}/test")
69+
expect(response.body).to eq("http://#{ADDRESS}:#{@port}/test")
6670
end
6771

6872
# context do

0 commit comments

Comments
 (0)