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

KubernetesTest / NamespaceExtension will run for every test class in the module #5315

Closed
shawkins opened this issue Jul 6, 2023 · 12 comments · Fixed by #5334
Closed

KubernetesTest / NamespaceExtension will run for every test class in the module #5315

shawkins opened this issue Jul 6, 2023 · 12 comments · Fixed by #5334
Assignees
Labels
Milestone

Comments

@shawkins
Copy link
Contributor

shawkins commented Jul 6, 2023

As long as a single test class is annotated, KubernetesTest / NamespaceExtension will run for every test class in the module regardless of whether the test is annotated.

@manusa
Copy link
Member

manusa commented Jul 11, 2023

Haven't checked it, but this might be happening for the other Jupiter Extensions+Annotations too (in case it's a bug).

Also note:

https://github.com/fabric8io/kubernetes-client/blob/bf4d07fdc52b9f47b2ee54d9a9b6f74ddaad8e2a/junit/kubernetes-junit-jupiter/src/main/resources/META-INF/services/org.junit.jupiter.api.extension.Extension

and:

junit.jupiter.extensions.autodetection.enabled=true

Our tests are actually configured to automatically run these extensions automatically (In case you're reporting this for the Fabric8 Client and not for an external project).

If this is happening at a different project, make sure that the junit.jupiter.extensions.autodetection.enabledis not set to true, and check that the extensions are running for all test classes.

@manusa manusa added the Waiting on feedback Issues that require feedback from User/Other community members label Jul 11, 2023
@shawkins
Copy link
Contributor Author

If this is happening at a different project, make sure that the junit.jupiter.extensions.autodetection.enabledis not set to true, and check that the extensions are running for all test classes.

This is running in keycloak. autodetection must already be enabled by quarkus. When autodetection is enabled, then the null check here https://github.com/fabric8io/kubernetes-client/blob/master/junit/kubernetes-junit-jupiter/src/main/java/io/fabric8/junit/jupiter/NamespaceExtension.java#L110 allows for the namespace to be created even if the test class is not annotated.

@manusa manusa removed the Waiting on feedback Issues that require feedback from User/Other community members label Jul 11, 2023
@manusa
Copy link
Member

manusa commented Jul 11, 2023

I don't see a good way to fix this upstream. This is mostly the problem with autodetection enabled, I've had this problem in other projects too (not sure if JUnit offers a good solution to just enable some or filter the enabled extensions).

The workaround is to actually add the annotation and disable the namespace creation, but this is ugly too.

@shawkins
Copy link
Contributor Author

So your expectation is that with autodetection enabled that every class, regardless of whether it has an annotation, should have the NamespaceExtension run?

@manusa
Copy link
Member

manusa commented Jul 11, 2023

Well, that's not my expectation, is what junit.jupiter.extensions.autodetection.enabled does. We could say the same about Quarkus or any other JUnit extensions that register as SPIs ;)

I've had problems like this with other projects. IMO if you start running into these issues you should disable autodetection.

The other alternative is to remove our SPI provided extensions and then annotate all of our tests. Or somewhat refactor this and create the SPI declaration in the test module.

@shawkins
Copy link
Contributor Author

@manusa
Copy link
Member

manusa commented Jul 11, 2023

Could https://github.com/fabric8io/kubernetes-client/blob/master/junit/kubernetes-junit-jupiter/src/main/java/io/fabric8/junit/jupiter/NamespaceExtension.java#L110 not create the namespace if the annotation is null?

I guess it could, but then we need to annotate our tests.

@shawkins
Copy link
Contributor Author

I guess it could, but then we need to annotate our tests.

Oh, I forgot about LoadKubernetesManifests - KubernetesTest is not required everywhere you'd expect the NamespaceExtension to be triggered. So either the extension logic would have to check for all such annotations, or keycloak will need annotate it's other test classes if we want to disable the namespace creation.

@manusa
Copy link
Member

manusa commented Jul 11, 2023

I think it's a matter of deciding if we stop populating the SPI file, produce it at a different module -kubernetes-junit-jupiter-automatic 🤷-, or suggesting downstream projects (in this case Keycloak) that they should disable automatic extension loading if they are running into issues.
We can discuss it tomorrow.

@shawkins
Copy link
Contributor Author

or suggesting downstream projects (in this case Keycloak) that they should disable automatic extension loading if they are running into issues

The trouble with that is quarkus seems to be implicitly relying on automatic extension loading, it's not a choice that keycloak is making directly.

@manusa
Copy link
Member

manusa commented Jul 11, 2023

The trouble with that is quarkus seems to be implicitly relying on automatic extension loading, it's not a choice that keycloak is making directly.

Which IMO is wrong on the Quarkus side. Even more considering that Quarkus is not 100% JUnit behavior compliant.

Anyway, the easiest move is to provide the junit-platform.properties in a different module (e.g. kubernetes-junit-jupiter-autodetected) and make our tests rely on this module instead. No need to annotate anything, downstream projects can either rely on the standard module or the autodetected one.

@shawkins
Copy link
Contributor Author

Which IMO is wrong on the Quarkus side. Even more considering that Quarkus is not 100% JUnit behavior compliant.

Here's where they are setting the default:

https://github.com/quarkusio/quarkus/blob/80826ecd747ac056847100e480ad778a9f6bc888/test-framework/junit5-properties/src/main/resources/junit-platform.properties#L1C1-L1C1

They talk about using a different one here https://github.com/quarkusio/quarkus/blob/main/docs/src/main/asciidoc/getting-started-testing.adoc#10-testing-different-profiles - just introducing another module will result in a warning, the quarkus one will need excluded. I'm not sure what QuarkusTest mechanisms may then not work correctly. I'll try it out locally and see.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants