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

[builder] make retries configurable for faster tests #10035

Merged
merged 2 commits into from
May 3, 2024

Conversation

kristinapathak
Copy link
Contributor

Description

When running tests, waiting for downloadModules() to fail 3 times when that's expected adds time to the test run. This updates tests to only attempt downloading once. Note: if there's a network failure that could cause downloadModules() to fail when it should normally succeed. Also the wording here is retries when in actuality it's the number of attempts. I didn't change this to keep the log wording the same, but I can change the wording if that's preferable.

Link to tracking issue

this will help for adding tests for #9252 and #9896

Testing

Tests ran

Copy link

codecov bot commented Apr 26, 2024

Codecov Report

Attention: Patch coverage is 71.42857% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 91.87%. Comparing base (326ef7c) to head (565a78c).
Report is 17 commits behind head on main.

Files Patch % Lines
cmd/builder/internal/builder/main.go 33.33% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10035      +/-   ##
==========================================
+ Coverage   91.55%   91.87%   +0.31%     
==========================================
  Files         360      360              
  Lines       16693    16728      +35     
==========================================
+ Hits        15284    15369      +85     
+ Misses       1073     1021      -52     
- Partials      336      338       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kristinapathak kristinapathak marked this pull request as ready for review April 26, 2024 15:18
@kristinapathak kristinapathak requested a review from a team as a code owner April 26, 2024 15:18
Copy link
Member

@mx-psi mx-psi 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! Just one very minor comment :)

cmd/builder/internal/builder/config.go Outdated Show resolved Hide resolved
@mx-psi mx-psi added the Skip Changelog PRs that do not require a CHANGELOG.md entry label May 2, 2024
Co-authored-by: Pablo Baeyens <pbaeyens31+github@gmail.com>
@mx-psi mx-psi merged commit ff7a485 into open-telemetry:main May 3, 2024
47 of 48 checks passed
@github-actions github-actions bot added this to the next release milestone May 3, 2024
@kristinapathak kristinapathak deleted the builder-faster-tests branch May 7, 2024 03:07
andrzej-stencel pushed a commit to andrzej-stencel/opentelemetry-collector that referenced this pull request May 27, 2024
…10035)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description
When running tests, waiting for `downloadModules()` to fail 3 times when
that's expected adds time to the test run. This updates tests to only
attempt downloading once. Note: if there's a network failure that could
cause `downloadModules()` to fail when it should normally succeed. Also
the wording here is `retries` when in actuality it's the number of
attempts. I didn't change this to keep the log wording the same, but I
can change the wording if that's preferable.

<!-- Issue number if applicable -->
#### Link to tracking issue
this will help for adding tests for
open-telemetry#9252
and
open-telemetry#9896

<!--Describe what testing was performed and which tests were added.-->
#### Testing
Tests ran

---------

Co-authored-by: Pablo Baeyens <pbaeyens31+github@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants