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

Fix for testutils.GetAvailablePort on Windows #1009

Conversation

james-bebbington
Copy link
Member

@james-bebbington james-bebbington commented May 21, 2020

On Windows, there is a significant wait when connecting to a local port that is not active. That causes the testutils.GetAvailablePort function, which has an assumption that a call to net.Dial will be very quick, to take much longer than the expected 5s (around 1 min).

See related discussion here: golang/go#23366

Re-implemented using a ticker with timeout channel so the function will terminate after 5s regardless of the number of attempts that were able to be made in that period.

@codecov
Copy link

codecov bot commented May 21, 2020

Codecov Report

Merging #1009 into master will increase coverage by 0.05%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1009      +/-   ##
==========================================
+ Coverage   85.63%   85.69%   +0.05%     
==========================================
  Files         188      188              
  Lines       13147    13152       +5     
==========================================
+ Hits        11259    11270      +11     
+ Misses       1437     1432       -5     
+ Partials      451      450       -1     
Impacted Files Coverage Δ
testutils/testutils.go 87.93% <100.00%> (+1.13%) ⬆️
translator/internaldata/resource_to_oc.go 76.47% <0.00%> (ø)
service/service.go 49.81% <0.00%> (+0.72%) ⬆️
...ceiver/hostmetricsreceiver/hostmetrics_receiver.go 73.52% <0.00%> (+2.94%) ⬆️
receiver/opencensusreceiver/octrace/opencensus.go 93.33% <0.00%> (+3.33%) ⬆️

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 36023bd...6be2d18. Read the comment docs.

@bogdandrutu bogdandrutu merged commit 3376f59 into open-telemetry:master May 21, 2020
wyTrivail pushed a commit to mxiamxia/opentelemetry-collector that referenced this pull request Jul 13, 2020
hughesjj pushed a commit to hughesjj/opentelemetry-collector that referenced this pull request Apr 27, 2023
…y#1009)

Bumps [boto3](https://github.com/boto/boto3) from 1.20.17 to 1.20.20.
- [Release notes](https://github.com/boto/boto3/releases)
- [Changelog](https://github.com/boto/boto3/blob/develop/CHANGELOG.rst)
- [Commits](boto/boto3@1.20.17...1.20.20)

---
updated-dependencies:
- dependency-name: boto3
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Troels51 pushed a commit to Troels51/opentelemetry-collector that referenced this pull request Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants