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

fixes flaky test #195

Merged
merged 1 commit into from
Oct 4, 2021
Merged

fixes flaky test #195

merged 1 commit into from
Oct 4, 2021

Conversation

squeedee
Copy link
Member

@squeedee squeedee commented Oct 4, 2021

Co-authored-by: Emily Johnson emjohnson@vmware.com

- just extends the timeout, hopefully that is enough.
- ultimately, story to use informers will make this test redundant
[#23]

Co-authored-by: Emily Johnson <emjohnson@vmware.com>
@netlify
Copy link

netlify bot commented Oct 4, 2021

✔️ Deploy Preview for elated-stonebraker-105904 canceled.

🔨 Explore the source changes: fee48dc

🔍 Inspect the deploy log: https://app.netlify.com/sites/elated-stonebraker-105904/deploys/615b4b22b30d9600070d63d2

@cirocosta
Copy link
Contributor

cirocosta commented Oct 4, 2021

> ultimately, story to use informers will make this test redundant

aah maybe I'm misunderstanding the test, but I thought that polling interval wasn't relevant because of how what we're testing here is all about shorticuiting the interval in the first place 🤔 isn't that the case?

Copy link
Contributor

@cirocosta cirocosta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@squeedee
Copy link
Member Author

squeedee commented Oct 4, 2021

> ultimately, story to use informers will make this test redundant

aah maybe I'm misunderstanding the test, but I thought that polling interval wasn't relevant because of how what we're testing here is all about shorticuiting the interval in the first place 🤔 isn't that the case?

The way I read it is, this test ensures that backoff is shortcut by observing a supply chain change. It's really simple to prove that it's applied but not that the component chain's speed is reset to "fast"

If the speed isn't reset, then any component affected by the supply chain change will immediately restamp. but we might still wait hours to copy results down the chain. Happy to be corrected.

So assuming I'm right about the above, then there's no concerns about backoff when we use informers instead, because updates to the stamped resource trigger a reconcile

@squeedee squeedee merged commit 0ae750f into main Oct 4, 2021
@squeedee squeedee deleted the 111-fix-flake branch October 4, 2021 21:06
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