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

🐛 increase eventually timeout to reduce false negatives #1619

Conversation

perdasilva
Copy link
Contributor

Description

I've been noticing a few false negatives on the upgrade-e2e. Increasing the eventually timeout to see if that helps reduce the incidence.

Reviewer Checklist

  • API Go Documentation
  • Tests: Unit Tests (and E2E Tests, if appropriate)
  • Comprehensive Commit Messages
  • Links to related GitHub Issue(s)

@perdasilva perdasilva requested a review from a team as a code owner January 15, 2025 09:11
Copy link

netlify bot commented Jan 15, 2025

Deploy Preview for olmv1 ready!

Name Link
🔨 Latest commit 7bed01a
🔍 Latest deploy log https://app.netlify.com/sites/olmv1/deploys/67877bd42fb13e00089b0d96
😎 Deploy Preview https://deploy-preview-1619--olmv1.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

codecov bot commented Jan 15, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.68%. Comparing base (2c8e3b9) to head (7bed01a).
Report is 5 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1619   +/-   ##
=======================================
  Coverage   66.68%   66.68%           
=======================================
  Files          57       57           
  Lines        4584     4584           
=======================================
  Hits         3057     3057           
  Misses       1302     1302           
  Partials      225      225           
Flag Coverage Δ
e2e 52.72% <ø> (+0.08%) ⬆️
unit 53.62% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@azych
Copy link
Contributor

azych commented Jan 15, 2025

From what I can see there was actually already a PR created by @camilamacedo86 to do this but it was closed because the consensus was to do a deeper investigation and try to fix the underlying problem.
This is the issue which also links to previous PR - #1550

Having said that, I notice that those failures are getting more and more common, so from my POV it might be a good idea to have the timeout increased until proper fix is in place.

@perdasilva
Copy link
Contributor Author

From what I can see there was actually already a PR created by @camilamacedo86 to do this but it was closed because the consensus was to do a deeper investigation and try to fix the underlying problem. This is the issue which also links to previous PR - #1550

Having said that, I notice that those failures are getting more and more common, so from my POV it might be a good idea to have the timeout increased until proper fix is in place.

Thanks for that! Let's see what the team reckons. Maybe we could create a tracking issue, though then we lose the forcing function to RC...

@perdasilva perdasilva marked this pull request as draft January 15, 2025 11:10
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 15, 2025
@perdasilva
Copy link
Contributor Author

Moving to draft to block merging until we have a wider discussion with the community/team

@camilamacedo86
Copy link
Contributor

Hi @perdasilva

Yep, we have the issue for it already: #1550
The only test that fails is this one. The issue only occurs if we have the e2e test running for more than 1 github action at the time.

So, I would suggest we close this one until the analysis/issue be addressed. WDYT?

@perdasilva perdasilva closed this Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants