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

[Merged by Bors] - Test failing CI tests due to port conflicts #4134

Closed
wants to merge 4 commits into from

Conversation

jimmygchen
Copy link
Member

@jimmygchen jimmygchen commented Mar 27, 2023

Issue Addressed

#4127. PR to test port conflicts in CI tests .

Proposed Changes

See issue for more details, potential solution could be adding a cache bound by time to the unused_port function.

@jimmygchen jimmygchen changed the title Test Test failing CI tests due to port conflicts Mar 27, 2023
@jimmygchen jimmygchen self-assigned this Mar 27, 2023
@jimmygchen jimmygchen changed the base branch from stable to unstable March 28, 2023 04:53
@jimmygchen
Copy link
Member Author

jimmygchen commented Mar 28, 2023

@antondlr this seems to fix the "Address in use" issue on CI - I've ran it 6 times on the self hosted large runner without errors.

@jimmygchen jimmygchen added ready-for-review The code is ready for review test improvement Improve tests and removed do-not-merge labels Mar 28, 2023
@michaelsproul
Copy link
Member

This looks good to me, but did I see somewhere that there were issues with socket reuse?

@jimmygchen
Copy link
Member Author

This looks good to me, but did I see somewhere that there were issues with socket reuse?

That was probably from me a few days ago, was having issue running it locally, but it seems to work well on CI. I'll need to figure out what's the issue.. could be a MacOS thing

Copy link
Collaborator

@divagant-martian divagant-martian left a comment

Choose a reason for hiding this comment

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

LGTM!

@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Mar 30, 2023
@michaelsproul
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Mar 30, 2023
## Issue Addressed

#4127. PR to test port conflicts in CI tests . 

## Proposed Changes

See issue for more details, potential solution could be adding a cache bound by time to the `unused_port` function.
@bors bors bot changed the title Test failing CI tests due to port conflicts [Merged by Bors] - Test failing CI tests due to port conflicts Mar 30, 2023
@bors bors bot closed this Mar 30, 2023
ghost pushed a commit to oone-world/lighthouse that referenced this pull request Jul 13, 2023
## Issue Addressed

sigp#4127. PR to test port conflicts in CI tests . 

## Proposed Changes

See issue for more details, potential solution could be adding a cache bound by time to the `unused_port` function.
Woodpile37 pushed a commit to Woodpile37/lighthouse that referenced this pull request Jan 6, 2024
## Issue Addressed

sigp#4127. PR to test port conflicts in CI tests . 

## Proposed Changes

See issue for more details, potential solution could be adding a cache bound by time to the `unused_port` function.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge This PR is ready to merge. test improvement Improve tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants