-
Notifications
You must be signed in to change notification settings - Fork 593
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
test: add timeout to context to trigger cleanup on wait for env ready #3894
Conversation
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## main #3894 +/- ##
=====================================
Coverage 59.0% 59.1%
=====================================
Files 138 138
Lines 15965 15965
=====================================
+ Hits 9434 9446 +12
+ Misses 5890 5881 -9
+ Partials 641 638 -3 see 4 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
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.
While this looks good, my prior testing proved that when we want to get the diagnostics from the cluster that failed to get ready there's not much that can be obtained because kubectl
invocations will fail.
It might help in case addons didn't get ready in time.
I think these should be added on KTF side to dump logs of |
I don't believe we have an issue for this. |
Kong/kubernetes-testing-framework#591 Does this PR help? |
It might, I'm not sure though. |
What this PR does / why we need it:
Add a
timeout
to context passed intoenv.WaitForReady
to trigger returning errors and cleanup, to avoid the timeout of test command that will not call the env cleanup, thus no diagnostics are outputted.Which issue this PR fixes:
Special notes for your reviewer:
PR Readiness Checklist:
Complete these before marking the PR as
ready to review
:[ ] theJust affect integration testsCHANGELOG.md
release notes have been updated to reflect any significant (and particularly user-facing) changes introduced by this PR