Skip to content

Conversation

camilamacedo86
Copy link
Contributor

@camilamacedo86 camilamacedo86 commented Jan 7, 2025

Increasing the timeout to avoid flakes

PS.: I notice that it sometimes fails in GitHub action; therefore, I am just increasing to avoid flakes.
Example: https://github.com/operator-framework/operator-controller/actions/runs/12653708386/job/35259771016?pr=1542

(I think it occurs when the same action is running and we push more changes )

@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 7, 2025
@camilamacedo86 camilamacedo86 marked this pull request as ready for review January 7, 2025 14:11
@camilamacedo86 camilamacedo86 requested a review from a team as a code owner January 7, 2025 14:11
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 7, 2025
Copy link

netlify bot commented Jan 7, 2025

Deploy Preview for olmv1 ready!

Name Link
🔨 Latest commit 2fb2150
🔍 Latest deploy log https://app.netlify.com/sites/olmv1/deploys/677d3628ccfb65000825c2cf
😎 Deploy Preview https://deploy-preview-1548--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 7, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.25%. Comparing base (83b01c3) to head (2fb2150).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1548   +/-   ##
=======================================
  Coverage   74.25%   74.25%           
=======================================
  Files          42       42           
  Lines        3329     3329           
=======================================
  Hits         2472     2472           
  Misses        676      676           
  Partials      181      181           
Flag Coverage Δ
e2e 52.02% <ø> (-0.10%) ⬇️
unit 56.98% <ø> (ø)

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.

@joelanford
Copy link
Member

joelanford commented Jan 7, 2025

One thing I think all reviewers should ask when a PR comes in for bumping a timeout (so I'll ask): what exactly is taking more than a minute when the flakes happen? Is there a different change we could make such that the test would execute within the existing time budget?

To me, there's a bit of a smell if it takes 3 minutes for an upgrade to happen.

Related question: should we set some SLO's for "how long do we expect it to take for an install/upgrade to happen?" And then start measuring that, and then make sure we don't regress.

@tmshort
Copy link
Contributor

tmshort commented Jan 7, 2025

Bumping timeouts is a race you can't win... We should be figuring out why it's taking so long and correcting that. e.g. We had flakes where the problem was that the older test resources hadn't been fully cleaned up. Bumping the timeouts on subsequent tests was not the answer. Waiting on those older resources to be cleaned up made subsequent test runs work in a reasonable amount of time.

@camilamacedo86
Copy link
Contributor Author

IMO, 1 minute is too low for this test.
I stopped to see the flake with 3, but 2 seems reasonable.
However, I closed this PR as not accepted and tracked an issue with the discussion summary so we can plan any further investigation since it is appropriately required.

@camilamacedo86
Copy link
Contributor Author

Done : #1550

Closing this one in favor of the issue

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.

3 participants