Skip to content

fix: add missing waits for httproute#985

Open
averevki wants to merge 1 commit into
Kuadrant:mainfrom
averevki:httproute-waits
Open

fix: add missing waits for httproute#985
averevki wants to merge 1 commit into
Kuadrant:mainfrom
averevki:httproute-waits

Conversation

@averevki
Copy link
Copy Markdown
Member

@averevki averevki commented May 18, 2026

Summary

Add missing waits for httproute. Adding the missing wait on the route fixture in singlecluster's conftest triggered a few sporadic testsuite failures, so I made a follow-up issue for it: #984

Summary by CodeRabbit

  • Tests
    • Enhanced test fixture configuration across multiple test suites with explicit readiness synchronisation steps for routes. This ensures all routes are fully ready before test execution begins, significantly improving test stability and preventing potential race condition issues that could occur during the test setup phase.

Review Change Stack

Signed-off-by: averevki <sandyverevkin@gmail.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 18, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2200aaa3-a6ba-4c32-b76e-819127f4baa3

📥 Commits

Reviewing files that changed from the base of the PR and between b70abe2 and ca217d5.

📒 Files selected for processing (4)
  • testsuite/tests/singlecluster/grpc/conftest.py
  • testsuite/tests/singlecluster/grpc/test_grpcroute_rule_targeting.py
  • testsuite/tests/singlecluster/identical_hostnames/conftest.py
  • testsuite/tests/singlecluster/ui/console_plugin/topology/conftest.py

📝 Walkthrough

Walkthrough

This PR adds route readiness synchronisation to four test fixture configurations. Each fixture now waits for its created route instance to become ready before returning it, ensuring consistent test setup sequencing across gRPC, HTTP route, and topology test suites.

Changes

Test fixture readiness synchronisation

Layer / File(s) Summary
Route readiness waits in pytest fixtures
testsuite/tests/singlecluster/grpc/conftest.py, testsuite/tests/singlecluster/grpc/test_grpcroute_rule_targeting.py, testsuite/tests/singlecluster/identical_hostnames/conftest.py, testsuite/tests/singlecluster/ui/console_plugin/topology/conftest.py
Four pytest fixtures for GRPCRoute and HTTPRoute now call wait_for_ready() after route creation and commit, ensuring resources are ready before tests execute.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related issues

Possibly related PRs

  • Kuadrant/testsuite#958: Modifies GRPCRoute test fixture setup in the same files by introducing infrastructure changes to the route creation and commit flow.

Suggested labels

test case, UI

Suggested reviewers

  • crstrn13
  • emmaaroche

Poem

🐰 Four fixtures now pause to wait,
For routes to reach a ready state,
No more racing ahead so fast,
With synchronisation here to last! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is incomplete, missing required sections (Changes and Verification) specified in the repository template. Expand the description to include a Changes section listing the modified test fixtures and a Verification section documenting how the changes were tested.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding missing waits for HTTPRoute fixtures across multiple test configuration files.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@averevki averevki requested review from a team and emmaaroche May 18, 2026 14:35
@averevki averevki self-assigned this May 18, 2026
@averevki averevki added bug Something isn't working refactor Refactor with same functionality labels May 18, 2026
Copy link
Copy Markdown
Contributor

@emmaaroche emmaaroche left a comment

Choose a reason for hiding this comment

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

Should this PR also cover the multicluster route fixture? The routes fixture in testsuite/tests/multicluster/conftest.py:94 commits without wait_for_ready().

Just a question / not a blocker though, still approving!

@emmaaroche
Copy link
Copy Markdown
Contributor

To follow up on my last comment these httproute fixtures also do not currently have a wait_for_ready:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working refactor Refactor with same functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants