Fix potential flaky test #1350
Merged
Fix potential flaky test #1350
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
ivoanjo
added a commit
that referenced
this pull request
Feb 4, 2021
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. This is the same issue as fixed in #1350 (both tests are similar). 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
marcotc
approved these changes
Feb 4, 2021
|
The broken test on JRuby is unrelated and was fixed on #1353 . Merging. |
ivoanjo
added a commit
that referenced
this pull request
Feb 9, 2021
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. This is the same issue as fixed in #1350 (both tests are similar). 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
ivoanjo
added a commit
that referenced
this pull request
Mar 11, 2021
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. This is the same issue as fixed in #1350 (both tests are similar). 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
ivoanjo
added a commit
that referenced
this pull request
Mar 16, 2021
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. This is the same issue as fixed in #1350 (both tests are similar). 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
ivoanjo
added a commit
that referenced
this pull request
Mar 29, 2021
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. This is the same issue as fixed in #1350 (both tests are similar). 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
ivoanjo
added a commit
that referenced
this pull request
Apr 12, 2021
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. This is the same issue as fixed in #1350 (both tests are similar). 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 [::]:6218due 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:
rspec_ngem (thanks @roberts1000 !) was very useful in running a given test case multiple times to cause it to triggersleep 1to the webrick shutdown sequence, thus making sure the background thread always gets delayed