QuarkusTransaction not working in Unit-Test since 2.16

[lostiniceland]

Describe the bug

We came across a regression when moving from Quarkus 2.15.3 to 2.16.4 where the static QuarkusTransaction calls fail when running in a unit-test (without a container) giving java.lang.NullPointerException: Cannot invoke "io.quarkus.arc.ArcContainer.instance(java.lang.Class, java.lang.annotation.Annotation[])" because the return value of "io.quarkus.arc.Arc.container()" is null

As stated in the documentation, QuarkusTransaction is supposed to work everywhere, using a no-op when no TX is available. see https://quarkus.io/guides/transaction

Expected behavior

Unit-Test runs to completion

Actual behavior

NullpointerException is thrown

How to Reproduce?

Define Bean with TX-semantics

@Singleton
public class TxBean {
    public void doStuff(){
        // using deprecated version to switch between versions. New api yields the same error
        QuarkusTransaction.run(() -> System.out.println("Hello TX"));
    }
}

Define a unit-test

public class TxBeanTest {

    @Test
    public void test(){
        var sut = new TxBean();
        sut.doStuff();
    }
}

Run it -> fails

Change version back of Quarkus to 2.15.3
Run again -> Success

Output of uname -a or ver

No response

Output of java -version

openjdk version "17.0.6" 2023-01-17

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.16.4

Build tool (ie. output of mvnw --version or gradlew --version)

Apache Maven 3.9.1

Additional information

As a workaround falling back to method annotated with @Transactional solves the issue

[lostiniceland]

Sample
quarkus-tx-32067.zip

[geoand]

cc @yrodiere

[yrodiere]

Thanks for the report.

This is probably a consequence of #31036 .

As stated in the documentation, QuarkusTransaction is supposed to work everywhere, using a no-op when no TX is available

Can you quote the exact sentence that gives this impression? I may need to clarify that.

Regardless, in your case the problem is not that no TX or TX manager is available; the CDI container, and with it the whole Quarkus infrastructure, is missing.

Presumably, this was working before because we relied on some static methods to get the transaction manager, and for some reason there was a transaction manager in your environment.

I suppose we could default to com.arjuna.ats.jta.TransactionManager.transactionManager() to get the transaction manager when we can't find CDI, but... I'm not sure we want to open that can of worms?

@geoand @mkouba , WDYT? Should we try to support that kind of things to any extent?

[lostiniceland]

See https://quarkus.io/guides/transaction#why-always-having-a-transaction-manager

Does it work everywhere I want to? Yep, it works in your Quarkus application, in your IDE, in your tests, because all of these are Quarkus applications. JTA has some bad press for some people. I don’t know why. Let’s just say that this is not your grandpa’s JTA implementation. What we have is perfectly embeddable and lean.

I was curious how this actually can work in a unit-test at all, since as you said you need a container. On the other hand, Quarkus does a lot of compile-time magic so I expected that the container call is replaced with some no-op when running in a test.

I actually prefer this style of managing transactions when fine grained control is needed. In JEE you always need to create an additional proxied method (and you can get lost in transactions if you are not careful) or inject TransactionManager which then gives you CheckedExceptions. Both are awkward.

[geoand]

@geoand @mkouba , WDYT? Should we try to support that kind of things to any extent?

I think that the use case described in this issue (where the "bean" is created manually) should not be supported, if it worked before it worked by accident.

[yrodiere]

See https://quarkus.io/guides/transaction#why-always-having-a-transaction-manager

Does it work everywhere I want to? Yep, it works in your Quarkus application, in your IDE, in your tests, because all of these are Quarkus applications. JTA has some bad press for some people. I don’t know why. Let’s just say that this is not your grandpa’s JTA implementation. What we have is perfectly embeddable and lean.

Ah, I see. I don't think this referred to unit tests since it mentions "because all of these are Quarkus applications". A unit test is not a Quarkus application, that's kind of the point of a unit test. I think this was more aimed at people who think JTA is hard to set up... Though I can't say for sure, I'm not the author of these lines.

@geoand @mkouba , WDYT? Should we try to support that kind of things to any extent?

I think that the use case described in this issue (where the "bean" is created manually) should not be supported, if it worked before it worked by accident.

I tend to agree... If we start to support using any CDI-related stuff without CDI available, we'll have tons of things to adapt. And transaction management in Quarkus is very much CDI-related since we have a transaction scope in CDI.

@lostiniceland I can offer two alternatives:

1. Run your test with @QuarkusTest and actually inject your bean into the test, like this. Though obviously it will become questionable whether this is still a "unit" test.
2. Set up a CDI context manually in your test, i.e. start Arc manually before your unit test and stop it after your test, making sure to include the bean definitions that you need (in this case, a mock jakarta.transaction.TransactionManager that will do what you want).

For 2., I'm not sure how easy it would be to start Arc manually with an explicit list of bean definitions. @mkouba may have some insights.

I wonder though... wouldn't it be useful in general to have a type of test where we only initialize CDI with a bunch of (mocked) beans, and developers can test their own bean in isolation? Some kind of @QuarkusUnitTest but being actually a unit test (+ CDI)?

Note that what we currently have with io.quarkus.test.QuarkusUnitTest is not a good match because 1. it's internal and 2. it's not a unit test at all, it starts a whole application.

[geoand]

Some kind of @QuarkusUnitTest but being actually a unit test (+ CDI)?

Although I get where this is coming from, I would very much prefer to avoid adding another type of test as I think it will just be too confusing for folks (not to mention that it will increase the aready heavy burden on maintaining test code).

[mkouba]

@geoand @mkouba , WDYT? Should we try to support that kind of things to any extent?

I think that the use case described in this issue (where the "bean" is created manually) should not be supported, if it worked before it worked by accident.

I fully agree.

For 2., I'm not sure how easy it would be to start Arc manually with an explicit list of bean definitions. @mkouba may have some insights.

Not so easy. Also even if we start the CDI container and provide a way to define the set of "active" application beans then how does a user find out which extension beans/services need to be mocked? Trial and error method?

I'm not saying it's impossible. But it would be more than "starting a CDI container"...

CC @manovotn @Ladicek

In any case, we should not throw a NPE but log an actionable error message if possible.

[manovotn]

I think that the use case described in this issue (where the "bean" is created manually) should not be supported, if it worked before it worked by accident.

I agree.

Not so easy. Also even if we start the CDI container and provide a way to define the set of "active" application beans then how does a user find out which extension beans/services need to be mocked? Trial and error method?

This is starting to sound a lot like Weld SE, or Weld JUnit :-)
However, in this case I am not sure you can even mock all the extensions as you might not even be aware of which are present or how to properly mock them in the first place.

Is there a reason why @QuarkusTest is insufficient or a bad choice in this case?

[yrodiere]

how does a user find out which extension beans/services need to be mocked? Trial and error method?

Well in this specific case it's just the transaction manager, so it's relatively straightforward. And yes, worst case, for a unit test trial and error is conceivable as the scope is limited.

Is there a reason why @QuarkusTest is insufficient or a bad choice in this case?

Well you can't easily use that in a library, for example. That's why unit tests could be useful. @QuarkusTest is not about unit testing, it starts the whole application.

But yes, I mentioned it as the first solution, and it could work depending on @lostiniceland's exact situation.

[lostiniceland]

Considering the alternatives, I guess we will have to work without QuarkusTransaction. We only use QuarkusTest for integration-test, meaning that all units have a Spock unit-test and only some (like modules) are tested with a running application.
Thats why I was so exited about QuarkusTransaction because it was a nice programmatic solution that (accidentally) worked in UTs as well :-)

[yrodiere]

Considering the alternatives, I guess we will have to work without QuarkusTransaction

I suppose you can also build a little wrapper around QuarkusTransaction and make it default to no-op if Arc.container() returns null. Not great, but may be better than alternatives.

[lostiniceland]

I suppose you can also build a little wrapper around QuarkusTransaction and make it default to no-op if Arc.container() returns null. Not great, but may be better than alternatives.

But when putting it behind a mockable wrapper, the question remains why I should use it at all. I could then just also wrap the TransactionManager, though the API is worse (CheckedException, Commit, Rollback etc)...but only in one place which aleviates the pain a bit

[mkouba]

how does a user find out which extension beans/services need to be mocked? Trial and error method?

Well in this specific case it's just the transaction manager, so it's relatively straightforward. And yes, worst case, for a unit test trial and error is conceivable as the scope is limited.

The scope is limited but you never know what other services are used in the dependency tree. Also not all extensions use CDI everywhere.

[yrodiere]

The scope is limited but you never know what other services are used in the dependency tree. Also not all extensions use CDI everywhere.

It's a unit test, so only direct dependencies matter and they're supposed to be stubbed or mocked. So yes, you'll know what other services you need quickly enough through trial and error.

Anyway... The consensus seems to be that the specific behavior requested in this should not be supported, and adding more testing extensions for actual unit tests is not desirable at the moment. So I'll close this.

Thanks for the input everyone!

[mkouba]

It's a unit test, so only direct dependencies matter and they're supposed to be stubbed or mocked.

I'm still a bit confused. If only direct dependencies matter then why use the CDI container at all? You could just use constructor/initializer injection points in your bean and mock anything you need... 🤷

[yrodiere]

I mean CDI dependencies.

Your bean might call a static method in Quarkus (say, QuarkusTransaction.run) that retrieves a CDI bean dynamically (Arc.container().whatever). You care about that dependency, and you'll easily notice it by running your test and looking at the failure, and then you'll mock it.

But you don't care about that dependency's transitive dependencies, because you mocked it, hopefully removing all transitive CDI dependencies.

[Ladicek]

I still think we should have a unit testing story. We've been hearing this constantly and don't have a good answer at the moment. QuarkusUnitTest is great for certain things, but it's relatively slow compared to "just" booting the CDI container. Something like Weld JUnit, e.g. ArcTestContainer or the ArC Arquillian adapter, would be a good fit for a lot of unit testing cases, I think.

[mkouba]

I still think we should have a unit testing story.

Well, it's not exactly "unit testing" but something more like "service mocking", right? Also I'm not sure that CDI is enough. I mean we would need to force all extensions/libs to use only CDI so that it's possible to mock all dependencies. For example, you would not be able to to org.eclipse.microprofile.config.ConfigProvider.getConfig().getValue() but @Inject @ConfigProperty instead. Speaking of configuration - we would need some convenient way to mock the config properties etc.

That said, I'm not against this idea!

[Ladicek]

One nice thing about Weld JUnit is that you can declaratively apply extensions, which lets you use e.g. MP Config easily. Unfortunately that is only possible with CDI extensions and not with Quarkus extensions -- supporting those basically amounts to the full QuarkusUnitTest. So you're right that there's a lot more constraints there, at least right now.

[mkouba]

One nice thing about Weld JUnit is that you can declaratively apply extensions, which lets you use e.g. MP Config easily.

Except that Quarkus config is a bit more complicated (we have ConfigPhase, LaunchMode, etc.). But yeah, I agree that weld-junit is a nice solution ;-).

So you're right that there's a lot more constraints there, at least right now.

Do we actually have a relevant feature request? If not then we should create one and discuss the ideas there...

[Ladicek]

I don't think we have a specific feature request for "unit testing", but this has been mentioned on many places over Zulip and GitHub issues. I found these 3 with just one search:

• Disabling starting Quarkus in QuarkusTests #5310
• Faster startup of @QuarkusTest (disable web-server?) #14927
• Allow certain features and extensions to be turned off while testing #23175