Skip to content
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.

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

Avoid EADDRINUSE from allmydata.test.test_system.SystemTest #518

Merged

Conversation

exarkun
Copy link
Member

@exarkun exarkun commented Aug 7, 2018

This replaces the use of iptutil.allocate_tcp_port which is inherently race prone with a strategy employing pre-allocated sockets.

Because the affected tests are complex and I had difficulty understanding how they work and what they are trying to accomplish, I also refactored them somewhat before starting. The changesets in this PR can largely be reviewed independently and doing so will make the significant changes I made to fix the issue (as opposed to refactoring) more readily apparent.

Fixes https://tahoe-lafs.org/trac/tahoe-lafs/ticket/2933

@codecov-io
Copy link

codecov-io commented Aug 8, 2018

Codecov Report

Merging #518 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #518      +/-   ##
=========================================
- Coverage    90.2%   90.2%   -0.01%     
=========================================
  Files         143     143              
  Lines       26267   26267              
  Branches     3699    3699              
=========================================
- Hits        23695   23694       -1     
  Misses       1871    1871              
- Partials      701     702       +1
Impacted Files Coverage Δ
src/allmydata/web/filenode.py 93.93% <0%> (-0.31%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cc3897a...f9d527f. Read the comment docs.

@exarkun
Copy link
Member Author

exarkun commented Aug 8, 2018

Coverage difference looks spurious to me - reported line is in some time-sensitive web handling code that presumably has a race as to whether it is covered or not in any particular test run.

@exarkun
Copy link
Member Author

exarkun commented Aug 22, 2018

Got a thumbs up from @MostAwesomeDude on IRC for this.

@exarkun exarkun merged commit 292448a into tahoe-lafs:master Aug 22, 2018
@exarkun exarkun deleted the test_upload_and_download_random_key branch August 22, 2018 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants