Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom

Use ephemeral ports in proxy tests #13234

Merged
merged 2 commits into from
Feb 23, 2025
Merged

Use ephemeral ports in proxy tests #13234

merged 2 commits into from
Feb 23, 2025

Conversation

sbidoul
Copy link
Member

@sbidoul sbidoul commented Feb 23, 2025 •
edited
Loading

This fixes a source of CI instability.

Yes I've seen "address already in use" in CI, and I happen to be in a mood to fix this :)

@sbidoul sbidoul added skip news Does not need a NEWS file entry (eg: trivial changes) type: maintenance Related to Development and Maintenance Processes labels Feb 23, 2025
@sbidoul sbidoul mentioned this pull request Feb 23, 2025
Merged
@sbidoul
Use ephemeral port in proxy tests
eacfb6b 
This is to avoid "address already in use" errors when running tests concurrently.
@sbidoul
Minor stylistic refactoring in proxy test
cbf2d30
@sbidoul sbidoul force-pushed the proxy-tests-ephemeral-ports-sbi branch from 9bbc9ce to cbf2d30 Compare February 23, 2025 11:42
ichard26
ichard26 approved these changes Feb 23, 2025
View reviewed changes
Copy link
Member

@ichard26 ichard26 left a comment

Choose a reason for hiding this comment

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

Thank you!

@ichard26
Copy link
Member

I can easily reproduce the OSError: [Errno 98] Address already in use locally and this patch does fix it.

@ichard26 ichard26 merged commit ae1b526 into pypa:main Feb 23, 2025
31 checks passed
@ichard26
Copy link
Member

I assume that you saw the CI error while taking a look at #13228 :)

@sbidoul sbidoul deleted the proxy-tests-ephemeral-ports-sbi branch February 23, 2025 17:03
@sbidoul
Copy link
Member Author

sbidoul commented Feb 23, 2025

I assume that you saw the CI error while taking a look at #13228 :)

Ah no, that was when doing #13232.

# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@notatallshaw notatallshaw notatallshaw approved these changes

@ichard26 ichard26 ichard26 approved these changes

Assignees
No one assigned
Labels
skip news Does not need a NEWS file entry (eg: trivial changes) type: maintenance Related to Development and Maintenance Processes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sbidoul @ichard26 @notatallshaw