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

e2e: replace sleep commands with wait directives of the related tools #711

Merged
merged 1 commit into from
Jun 12, 2021

Conversation

muvaf
Copy link
Member

@muvaf muvaf commented Jun 11, 2021

Description of your changes

We're seeing increase in e2e test failures due to conditions being not met after a specific duration. Increasing the timeout would just make it take longer because the checks were not done periodically. So, I changed it to run periodically and use the related tools' waiting mechanisms wherever possible, like helm's --wait flag and kubectl wait --timeout=60s command.

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

Manually with make e2e

@muvaf muvaf requested review from hasheddan and jbw976 June 11, 2021 09:03
Copy link
Member

@negz negz left a comment

Choose a reason for hiding this comment

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

LGTM at a glance, but I'll defer to Dan who wrote most of this.

I would like to ensure we propagate these changes to the other providers that were using these tests.

Copy link
Member

@hasheddan hasheddan left a comment

Choose a reason for hiding this comment

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

@muvaf this definitely seems like an improvement here, thanks! These tests were originally set up quickly and I think there are plenty of optimizations that can be made as part of the conformance effort. I'd also like for us to nail down what is causing the increased run time here, which may be related to package manager having more "resource has been modified" errors when installing CRDs. I'll investigate as a follow up 👍🏻

@muvaf muvaf merged commit 84acbf6 into crossplane-contrib:master Jun 12, 2021
@muvaf muvaf deleted the fix-e2e branch June 12, 2021 00:03
turkenh added a commit to turkenh/provider-sql that referenced this pull request Aug 25, 2021
Following crossplane-contrib/provider-aws#711

Signed-off-by: Hasan Turken <turkenh@gmail.com>
turkenh added a commit to turkenh/provider-helm that referenced this pull request Aug 25, 2021
Following crossplane-contrib/provider-aws#711

Signed-off-by: Hasan Turken <turkenh@gmail.com>
turkenh added a commit to turkenh/provider-sql that referenced this pull request Aug 27, 2021
Following crossplane-contrib/provider-aws#711

Signed-off-by: Hasan Turken <turkenh@gmail.com>
(cherry picked from commit 18b7c70)
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