-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
[chore] make contrib tests more efficient #12490
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #12490 +/- ##
==========================================
- Coverage 92.00% 91.99% -0.01%
==========================================
Files 469 469
Lines 25355 25355
==========================================
- Hits 23327 23325 -2
- Misses 1619 1620 +1
- Partials 409 410 +1 ☔ View full report in Codecov by Sentry. |
fc7ef2c
to
7cae2eb
Compare
7cae2eb
to
ed455e1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
9861e0a
contrib_path=/tmp/opentelemetry-collector-contrib | ||
git clone --depth=1 https://github.com/open-telemetry/opentelemetry-collector-contrib.git $contrib_path | ||
make CONTRIB_PATH=$contrib_path prepare-contrib | ||
- uses: actions/upload-artifact@v4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't it mean that we will be testing against one contrib version for 90 days?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm looking at https://github.com/actions/upload-artifact?tab=readme-ov-file#retention-period. Should we add git sha in the path
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ouch. I'll open a new PR to add that
Actually, no, it might not be a problem, since this happens every time and is used inside the same build. But we can certainly do it anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to upload it then? Should we use https://github.com/actions/cache instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I followed the recipe I have seen used most of the time. A cache is a step better I didn't look into because I was afraid of having cache issues. That would shave an additional 4 minutes off the build.
Instead of having each test group edit modules, tidy and generate, do it once and share the output in each subtest to save 5 minutes per branch.