search

arquillian / arquillian-extension-transaction

9 stars 10 forks source link

Fixed hiding of setup exception due to a NPE in TransactionHandler #9

Closed dashorst closed 9 years ago

dashorst commented 9 years ago

When the setup of a testcase fails due to some exception when setting up CDI context, the TransactionHandler throws a NullPointerException while trying to determine the transaction result (rollback or commit). This masks the original exception making it hard to figure out what goes wrong when building a context for test cases.

You can see the particular testcase here: https://github.com/dashorst/arquillian-transaction-npe-test

The commit in this PR fixes the NPE and doesn't hide the original exception

bartoszmajsak commented 9 years ago

Thanks a lot and apologies for keeping this PR hanging that long. I will bring it to master tonight.

How about bringing your test example to the test suite of the project as well?

dashorst commented 9 years ago

The issue is that the test case will always fail the build, even if the fix is in. I'm quite sure you don't want to have that in your automated build setup.

bartoszmajsak commented 9 years ago

But we can try to test expected exception, can't we? :)

dashorst commented 9 years ago

no, because in this particular test case the error happens outside the running of a test method. i.e. a setup failure injecting a CDI bean which is hidden by the NPE. This all is outside of the test method. If the test method were actually called, it has to fail since the setup completed without error.

aslakknutsen commented 9 years ago

You can always call JunitCore directly in a Junit @Test and assert on the Result..

botchniaque commented 9 years ago

Is the fix ever going to be merged?

bartoszmajsak commented 9 years ago

Yes

bartoszmajsak commented 9 years ago

Finally merged with master :) Thank you for the fix and patience!