Adding buffer to BeforeSuite tests#358
Adding buffer to BeforeSuite tests#358openshift-merge-bot[bot] merged 1 commit intoopendatahub-io:mainfrom
Conversation
|
Caution There are some errors in your PipelineRun template.
|
WalkthroughThe timeout for the Changes
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Signed-off-by: Mariah Holder <marholde@redhat.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
fvt/predictor/predictor_suite_test.go (3)
59-59: Remove commented code instead of leaving it in place.Commented-out code should be removed rather than left in the codebase, as version control already tracks the previous implementation.
- //WaitForStableActiveDeployState(time.Second * 45)
60-60: Improve log message clarity.The current log message "TIMEOUT 100" is not descriptive enough. Consider making it more informative about what is happening.
- Log.Info("TIMEOUT 100") + Log.Info("Waiting for stable deployment state with increased timeout of 100 seconds")
61-61: Consider using a named constant for the timeout value.The timeout change from 45 to 100 seconds addresses the CI deployment readiness issues as intended. However, consider extracting the timeout value into a named constant for better maintainability.
+const deploymentStabilityTimeout = 100 * time.Second + // ... later in the function - WaitForStableActiveDeployState(time.Second * 100) + WaitForStableActiveDeployState(deploymentStabilityTimeout)
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: andresllh, mholder6 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Added a buffer to the BeforeSuite tests to give the deployments more time to become ready in the CI tests.
Error:
[FAILED] in [SynchronizedBeforeSuite] - /go/src/github.com/opendatahub-io/modelmesh-serving/fvt/helpers.go:439 @ 06/25/25 16:19:34.265 [SynchronizedBeforeSuite] [FAILED] [52.262 seconds] [SynchronizedBeforeSuite] /go/src/github.com/opendatahub-io/modelmesh-serving/fvt/predictor/predictor_suite_test.go:34 [FAILED] Timed out before deployments were ready: map[modelmesh-serving-mlserver-1.x:false modelmesh-serving-ovms-1.x:false modelmesh-serving-torchserve-0.x:false modelmesh-serving-triton-2.x:false] Expected <bool>: false to be true In [SynchronizedBeforeSuite] at: /go/src/github.com/opendatahub-io/modelmesh-serving/fvt/helpers.go:439 @ 06/25/25 16:19:34.265 ------------------------------Summary by CodeRabbit