Skip to content

Commit

Permalink
Fix potential flaky test
Browse files Browse the repository at this point in the history
When starting webrick in-process in a background thread WITH a fixed
port, we need to make sure that the webrick thread has shut down
between tests, otherwise a follow-up test can fail with

`Errno::EADDRINUSE: Address already in use - bind(2) for [::]:6218`

due to the main test runner thread being faster at starting the next
test case before the old webrick thread has had time to shut down.

In this specific case, there's only a single test case for this file,
so this issue would only trigger whenever another test case also uses
webrick with a fixed port OR once more test cases are added to this
file.

I ran into this because there's a test in the profiling branch that
was partially copy-pasted from this one and had exactly the same issue,
so I'm fixing the issue on both places.

Tips for hunting down these kinds of issues:
* The `rspec_n` (thanks @roberts1000 !) was very useful in running
  a given test case multiple times to cause it to trigger
* This specific issue can be triggered really easily by modifying the
  webrick sources and adding a `sleep 1` to the webrick shutdown
  sequence, thus making sure the background thread always gets delayed
  • Loading branch information
ivoanjo committed Feb 4, 2021
1 parent 3f16b41 commit 89fabc8
Showing 1 changed file with 5 additions and 2 deletions.
7 changes: 5 additions & 2 deletions spec/ddtrace/transport/http/adapters/net_integration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,14 @@

before do
server.mount_proc('/', &server_proc)
Thread.new { server.start }
@server_thread = Thread.new { server.start }
init_signal.pop
end

after { server.shutdown }
after do
server.shutdown
@server_thread.join
end
end

describe 'when sending traces through Net::HTTP adapter' do
Expand Down

0 comments on commit 89fabc8

Please sign in to comment.