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

Remove references to removed WithOpenShiftTestServer functionality #45443

Merged
merged 1 commit into from
Jan 8, 2025

Conversation

manusa
Copy link
Contributor

@manusa manusa commented Jan 8, 2025

Description

Follows up on #45259 as part of #44899

Remove/Replace mentions in the kubernetes-client guide of OpenShift mockserver (quarkus-test-openshift-client).

Additionally added a test to ensure that OpenShift client injection from the Kubernetes MockServer works as expected.

/cc @metacosm

This comment has been minimized.

Copy link

github-actions bot commented Jan 8, 2025

🙈 The PR is closed and the preview is expired.

This comment has been minimized.

…Server functionality

Signed-off-by: Marc Nuri <marc@marcnuri.com>
@manusa manusa force-pushed the fix/openshift-mock-server branch from 414afa4 to a07da0c Compare January 8, 2025 14:39
Copy link

quarkus-bot bot commented Jan 8, 2025

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit a07da0c.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

Copy link

quarkus-bot bot commented Jan 8, 2025

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit a07da0c.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

@gsmet
Copy link
Member

gsmet commented Jan 8, 2025

@geoand Reading this, something interesting to note is that @WithKubernetesTestServer has restrictToAnnotatedClass = true which will be quite counter productive.

I'm not sure if we should change that or have two different annotations? Maybe @WithSharedKubernetesTestServer .

I noticed that while investigating the sbomer test suite slowness.

Also @manusa could you write some Asciidoc content for me to include in the migration guide? Thanks!

@geoand
Copy link
Contributor

geoand commented Jan 8, 2025

Reading this, something interesting to note is that @WithKubernetesTestServer has restrictToAnnotatedClass = true which will be quite counter productive.

Nice catch!

I'm not sure if we should change that or have two different annotations? Maybe @WithSharedKubernetesTestServer .

That's probably better!

@manusa
Copy link
Contributor Author

manusa commented Jan 8, 2025

I'm not sure if we should change that or have two different annotations? Maybe @WithSharedKubernetesTestServer .

It's funny because I actually spent some time trying to understand why there were two different MockWebServer instances spawning when I launched the test. I eventually realized that the other instance came from a different test which had an annotation that behaves as you describe.

The MockWebServer starts really quickly. IMO it's better to have an instance per class/suite, so the actual behavior of the WithKubernetesTestServer is what I'd expect (it aligns more with the behavior of the vanilla Fabric8 Kubernetes Client JUnit annotations).

So I'd vote for a @WithSharedKubernetesTestServer instead of changing the actual behavior.

Also @manusa could you write some Asciidoc content for me to include in the migration guide? Thanks!

Sure, I'll create some content and put it in the original issue #44899 and ping you there.

@gsmet
Copy link
Member

gsmet commented Jan 8, 2025

The MockWebServer starts really quickly. IMO it's better to have an instance per class/suite, so the actual behavior of the WithKubernetesTestServer is what I'd expect (it aligns more with the behavior of the vanilla Fabric8 Kubernetes Client JUnit annotations).

Yeah so it's not about the MockServer being slow, it's about potentially requiring a lot more Quarkus restarts when running the tests.

@gsmet gsmet merged commit dbdfcea into quarkusio:main Jan 8, 2025
23 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.18 - main milestone Jan 8, 2025
@manusa manusa deleted the fix/openshift-mock-server branch January 9, 2025 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants