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

@QuarkusComponentTest feedback #34926

Closed
manofthepeace opened this issue Jul 21, 2023 · 12 comments
Closed

@QuarkusComponentTest feedback #34926

manofthepeace opened this issue Jul 21, 2023 · 12 comments
Labels
area/testing kind/enhancement New feature or request triage/duplicate This issue or pull request already exists

Comments

@manofthepeace
Copy link
Contributor

Description

I started playing with the new @QuarkusComponentTest. At first glance I really do like it as it is really fast in comparison of QuarkusTest.

While using I found 2 things that made it a bit tedious when I first tried it.

1- In quarkus the no args constructor are generated, but QuarkusComponentTest requires it when there is already a non no-arg constructor present. (i.e. single constructor without the Inject annotation)

I am not sure this is actually possible to fix, but it would be nice if I did not have to change my sources to use that test strategy. Meaning it would be nice if it could auto-magically create the no arg ctor as quarkus does when running the an app.

This is the error I get while running the test is the following jakarta.enterprise.inject.spi.DeploymentException: Normal scoped beans must declare a non-private constructor with no parameters: CLASS bean [types=[org.acme.Foo, java.lang.Object], qualifiers=[@Default, @Any], target=org.acme.Foo]

code to reproduce is as follow; adding commented out ctor + the @Inject makes the test work, but the app works without it.

@ApplicationScoped
public class Foo {
    Charlie charlie;
    
    //Foo() {}
    
    //@Inject
    Foo(Charlie charlie) {
        this.charlie = charlie;
    }
    String ping() {
        return charlie.ping();
    }
}

2- When Using @InjectMock on a bean that is not needed for example changing the above Foo class like this;

@ApplicationScoped
public class Foo {

    String ping() {
        return "OK";
    }

}

If my test class still has the @InjectMock of the Charlie bean, a NPE will follow like this one; which make troubleshooting quite hard as it does not tell what the issue is nor where it is.

java.lang.NullPointerException: Cannot invoke "io.quarkus.arc.InjectableBean.getKind()" because the return value of "io.quarkus.arc.InstanceHandle.getBean()" is null
	at io.quarkus.test.component.QuarkusComponentTestExtension$FieldInjector.<init>(QuarkusComponentTestExtension.java:854)
	at io.quarkus.test.component.QuarkusComponentTestExtension.injectFields(QuarkusComponentTestExtension.java:807)
	at io.quarkus.test.component.QuarkusComponentTestExtension.postProcessTestInstance(QuarkusComponentTestExtension.java:234)
	at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.lambda$invokeTestInstancePostProcessors$10(ClassBasedTestDescriptor.java:377)
	at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.executeAndMaskThrowable(ClassBasedTestDescriptor.java:382)
	at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.lambda$invokeTestInstancePostProcessors$11(ClassBasedTestDescriptor.java:377)
	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
	at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:179)
	at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1625)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
	at java.base/java.util.stream.StreamSpliterators$WrappingSpliterator.forEachRemaining(StreamSpliterators.java:310)
	at java.base/java.util.stream.Streams$ConcatSpliterator.forEachRemaining(Streams.java:735)
	at java.base/java.util.stream.Streams$ConcatSpliterator.forEachRemaining(Streams.java:734)
	at java.base/java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:762)
	at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.invokeTestInstancePostProcessors(ClassBasedTestDescriptor.java:376)
	at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.lambda$instantiateAndPostProcessTestInstance$6(ClassBasedTestDescriptor.java:289)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.instantiateAndPostProcessTestInstance(ClassBasedTestDescriptor.java:288)
	at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.lambda$testInstancesProvider$4(ClassBasedTestDescriptor.java:278)
	at java.base/java.util.Optional.orElseGet(Optional.java:364)
	at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.lambda$testInstancesProvider$5(ClassBasedTestDescriptor.java:277)
	at org.junit.jupiter.engine.execution.TestInstancesProvider.getTestInstances(TestInstancesProvider.java:31)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$prepare$0(TestMethodTestDescriptor.java:105)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.prepare(TestMethodTestDescriptor.java:104)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.prepare(TestMethodTestDescriptor.java:68)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$prepare$2(NodeTestTask.java:123)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.prepare(NodeTestTask.java:123)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:90)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:35)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:54)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:147)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:127)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:90)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.lambda$execute$0(EngineExecutionOrchestrator.java:55)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.withInterceptedStreams(EngineExecutionOrchestrator.java:102)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:54)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:114)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:95)
	at org.junit.platform.launcher.core.DefaultLauncherSession$DelegatingLauncher.execute(DefaultLauncherSession.java:91)
	at org.junit.platform.launcher.core.SessionPerRequestLauncher.execute(SessionPerRequestLauncher.java:60)
	at org.eclipse.jdt.internal.junit5.runner.JUnit5TestReference.run(JUnit5TestReference.java:98)
	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:40)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:529)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:756)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:452)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:210)
	Suppressed: java.lang.NullPointerException: Cannot invoke "java.util.List.iterator()" because the return value of "org.junit.jupiter.api.extension.ExtensionContext$Store.get(Object, java.lang.Class)" is null
		at io.quarkus.test.component.QuarkusComponentTestExtension.preDestroyTestInstance(QuarkusComponentTestExtension.java:244)
		at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeAllAfterMethodsOrCallbacks$13(TestMethodTestDescriptor.java:276)
		at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
		at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeAllAfterMethodsOrCallbacks$14(TestMethodTestDescriptor.java:276)
		at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
		at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeAllAfterMethodsOrCallbacks(TestMethodTestDescriptor.java:275)
		at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeTestInstancePreDestroyCallbacks(TestMethodTestDescriptor.java:264)
		at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.cleanUp(TestMethodTestDescriptor.java:153)
		at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.cleanUp(TestMethodTestDescriptor.java:68)
		at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$cleanUp$10(NodeTestTask.java:167)
		at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
		at org.junit.platform.engine.support.hierarchical.NodeTestTask.cleanUp(NodeTestTask.java:167)
		at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:98)
		... 39 more


Implementation ideas

No response

@quarkus-bot
Copy link

quarkus-bot bot commented Jul 23, 2023

/cc @geoand (testing)

@manovotn
Copy link
Contributor

Also CC @mkouba @Ladicek

1- In quarkus the no args constructor are generated, but QuarkusComponentTest requires it when there is already a non no-arg constructor present. (i.e. single constructor without the Inject annotation)

I'd say this is expected given that we skip lots of Quarkus bootstrapping. In fact, I'd expect more of these functionalities that are there for convinience won't work because they mainly reside in Quarkus integration code whereas here we try to boot very minimal CDI (Arc) and some MP config with that.

2- When Using @InjectMock on a bean that is not needed for example changing the above Foo class like this;

That does look like a bug, thanks for reporting it back.

@geoand
Copy link
Contributor

geoand commented Jul 24, 2023

@manovotn even though 1 might be expected, it's definitely not a good used experience.
We should try and see if we can overcome this limitation

@Ladicek
Copy link
Contributor

Ladicek commented Jul 24, 2023

The issue 1 is not caused by "skipping Quarkus bootstrap" [1], it's because we don't do bytecode transformations in @QuarkusComponentTest. I once tried adding bytecode transformations to ArcTestContainer and it didn't end well -- I'm pretty sure it would be the same case here. Everything comes from the system classloader, and some classes the test class refers to may get transformed, so in the end, we'd have to do the same classloader dance that @QuarkusTest does.

[1] Skipping Quarkus bootstrap would cause other issues, though. For example, I think static methods won't be intercepted in @QuarkusComponentTest, because that is solely implemented in the Quarkus ArC extension and not in ArC itself. (And, of course, it also requires bytecode transformation, but that would be a secondary problem here :-) )

@manovotn
Copy link
Contributor

You're right, I meant to say we skip the bytecode transformation that full blown Quarkus normally does.

I think static methods won't be intercepted in @QuarkusComponentTest, because that is solely implemented in the Quarkus ArC extension and not in ArC itself. (And, of course, it also requires bytecode transformation, but that would be a secondary problem here :-) )

Yes, that's the other case I could think of.
I don't think we use transformation in other cases though; the rest should be via just annotation manipulation, right? (for instance adding @Inject or scopes to classes with producers but no bean defining annotation etc...)

@Ladicek
Copy link
Contributor

Ladicek commented Jul 24, 2023

I briefly looked at ArC and this is what we use bytecode transformations for:

  • transformation of unproxyable classes (removing final from classes and methods, adding a zero-param ctor when missing, changing a private zero-param ctor to package-private)
  • removing final from methods that are intercepted (to be able to generate a subclass) or that are on a normal-scoped bean (to be able to generate a client proxy)
  • transformation of private @Inject fields (to package-private)
  • bonus: removing final from custom AlterableContext classes (to be able to generate a subclass that implements InjectableContext)

@Ladicek
Copy link
Contributor

Ladicek commented Jul 28, 2023

I figured out how to support bytecode transformations in ArcTestContainer, so it should be as well possible with @QuarkusComponentTest. It requires the same classloader dance as @QuarkusTest, but that doesn't surprise anyone at this point I guess :-)

@mkouba
Copy link
Contributor

mkouba commented Jul 31, 2023

1- In quarkus the no args constructor are generated, but QuarkusComponentTest requires it when there is already a non no-arg constructor present. (i.e. single constructor without the Inject annotation)

We should be able to fix this thanks to @Ladicek's work. TBH I'm not very happy we'll introduce this kind of hack. But if it helps 🤷.

2- When Using @InjectMock on a bean that is not needed for example changing the above Foo class like this;

If my test class still has the @InjectMock of the Charlie bean, a NPE will follow like this one; which make troubleshooting quite hard as it does not tell what the issue is nor where it is.

You should get a different error in the main branch, something like The injected field X expects a mocked bean; but obtained null. But we should probably improve the error message a little bit ;-)

@ennishol
Copy link
Contributor

ennishol commented Mar 8, 2024

@manovotn even though 1 might be expected, it's definitely not a good used experience. We should try and see if we can overcome this limitation

I also tried the new extension and it is sort of disappointing that quarkus does support beans without a no-args constructor but @QuarkusComponentTest does not. Then maybe, the name should be @CdiComponentTest to stress that the quarkus feature is not supported here.

I understand that CDI specs require it, but it would be a better user experience if the @QuarkusComponentTest does match what the framework offers. I'm not sure about the other CDI based framework offer but comparing with spring it is really a limitation from the user perspective

Otherwise, it is an awesome extension :)

@Ladicek
Copy link
Contributor

Ladicek commented Mar 8, 2024

I had a pull request that allowed this, see #35473. It requires a fairly ugly "classloader dance", so I ended up closing it -- but that doesn't mean we don't want to do this. It is very much supposed to work (at least from my perspective :-) )

@holly-cummins is pursuing a holistic cleanup of classloader manipulation in the testing extensions, which should make this much less invasive. It's a long run though.

@ennishol
Copy link
Contributor

ennishol commented Mar 8, 2024

Thanks! Nice to know it was not forgotten or abandoned. Looking forward to the improvement!

@mkouba
Copy link
Contributor

mkouba commented Sep 17, 2024

This issue duplicates #43339.

@mkouba mkouba closed this as completed Sep 17, 2024
@geoand geoand added the triage/duplicate This issue or pull request already exists label Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/testing kind/enhancement New feature or request triage/duplicate This issue or pull request already exists
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants