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

Wait for Gateway to be ready before using it #523

Merged
merged 1 commit into from
Sep 5, 2024

Conversation

trepel
Copy link
Contributor

@trepel trepel commented Sep 4, 2024

Overview

This is a small try to further stabilze the test_preexisting_auth, see
#427

Verification Steps

Execute

flags="-vvv --standalone" make testsuite/tests/singlecluster/authorino/operator/sharding/test_preexisting_auth.py

and

flags="-vvv --standalone" make testsuite/tests/singlecluster/authorino/operator/sharding/test_sharding.py

Both should pass. We will see in nightlies whether this actually helped or not.

Copy link
Contributor

@azgabur azgabur left a comment

Choose a reason for hiding this comment

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

I tried to verify with running more tests concurrently as this is probably the race condition that triggers this bug

flags=-n20 make authorino-standalone

Sadly the test_preexisting_auth still occasionally fails so the problem must be somewhere deeper. I remember from talks with @pehala that this test is fundamentally wrong so maybe we need to look at the test case itself to determine if it is something we even need to test or if it has really been a bug in Authorino this whole time.

@trepel
Copy link
Contributor Author

trepel commented Sep 5, 2024

@azgabur Thanks for review! I think that this change makes logical sense anyway so I will merge this one. About the test itself please add what you know/recall that might help to #427

@trepel trepel merged commit df0ae69 into Kuadrant:main Sep 5, 2024
3 checks passed
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