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

[JBTM-3454] JTA CDI should roll-back when java.lang.Error is thrown from the @Transactional method #1812

Merged
merged 1 commit into from Apr 12, 2021

Conversation

ochaloup
Copy link
Contributor

@ochaloup ochaloup commented Apr 7, 2021

https://issues.redhat.com/browse/JBTM-3454

MAIN AS_TESTS
!QA_JTA !QA_JTS_JDKORB !QA_JTS_OPENJDKORB !QA_JTS_JACORB !BLACKTIE !XTS !PERF NO_WIN !RTS !TOMCAT !JACOCO !LRA

@jbosstm-bot
Copy link

@jbosstm-bot
Copy link

@jbosstm-bot
Copy link

@jbosstm-bot
Copy link

Copy link
Contributor

@mayankkunwar mayankkunwar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I ran this code in my local, I saw that one of the test was failing TransactionalImplTest.testDefaultThrowError:158 Should have rolled back, but committed https://github.com/jbosstm/narayana/pull/1812/files#diff-64830153cfb38fe8a4a86d4fb84937907b67c5e331899564c3a184afb4427257R145

And in the CI run I observed that, there is no result for ArjunaJTA/cdi module http://narayanaci1.eng.hst.ams2.redhat.com/view/Pulls/job/btny-pulls-narayana/10/PROFILE=CORE,jdk=jdk8.latest,label=linux/artifact/ArjunaJTA/

@ochaloup
Copy link
Contributor Author

ochaloup commented Apr 8, 2021

@mayankkunwar sure, the local failure will be (I assume as the most probably reason) caused by fact you haven't updated the jts modules in the WFLY distribution. Without the fixes such failure is expected.

Regarding the ArjunaJTA/cdi module - there are no results as the CI did not get to that phase. First the WFLY has to be built and only after that the -Parq integration tests are run - ie. see https://github.com/jbosstm/narayana/blob/master/scripts/hudson/narayana.sh#L1087
As the build failed there were no tests and not results.

I will rerun the CI as it seems the CI failure was the only trouble on downloading artifacts.

@jbosstm-bot
Copy link

@mayankkunwar
Copy link
Contributor

I see @ochaloup , thanks for the clarification.

@jbosstm-bot
Copy link

@ochaloup
Copy link
Contributor Author

ochaloup commented Apr 8, 2021

Again not related error. It was

[ERROR] Caused by: org.apache.maven.surefire.booter.SurefireBooterForkException: Error creating properties files for forking
[ERROR] 	at org.apache.maven.plugin.surefire.booterclient.ForkStarter.fork(ForkStarter.java:574)
[ERROR] 	at org.apache.maven.plugin.surefire.booterclient.ForkStarter.access$600(ForkStarter.java:115)
[ERROR] 	at org.apache.maven.plugin.surefire.booterclient.ForkStarter$2.call(ForkStarter.java:444)
[ERROR] 	at org.apache.maven.plugin.surefire.booterclient.ForkStarter$2.call(ForkStarter.java:420)
[ERROR] 	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
[ERROR] 	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
[ERROR] 	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
[ERROR] 	at java.lang.Thread.run(Thread.java:748)
[ERROR] Caused by: java.io.IOException: No space left on device
[ERROR] 	at java.io.UnixFileSystem.createFileExclusively(Native Method)
[ERROR] 	at java.io.File.createTempFile(File.java:2026)
[ERROR] 	at org.apache.maven.surefire.booter.SystemPropertyManager.writePropertiesFile(SystemPropertyManager.java:81)
[ERROR] 	at org.apache.maven.plugin.surefire.booterclient.BooterSerializer.serialize(BooterSerializer.java:182)
[ERROR] 	at org.apache.maven.plugin.surefire.booterclient.ForkStarter.fork(ForkStarter.java:553)
[ERROR] 	... 7 more

@jbosstm-bot
Copy link

@jbosstm-bot
Copy link

@mmusgrov
Copy link
Contributor

mmusgrov commented Apr 8, 2021

@ochaloup Mayank is reviewing this but I think it looks good (although I did not investigate the reason why AS_TESTS failed).

@mayankkunwar
Copy link
Contributor

For me also changes looks good. I tried to test with some nested scenarios, changes works well.

@mmusgrov
Copy link
Contributor

mmusgrov commented Apr 9, 2021

@mayankkunwar so once the failures are accounted for you should approve it.

@mayankkunwar
Copy link
Contributor

mayankkunwar commented Apr 12, 2021

@mayankkunwar so once the failures are accounted for you should approve it.

Sure @mmusgrov

@jbosstm-bot
Copy link

@jbosstm-bot
Copy link

@jbosstm-bot
Copy link

@ochaloup
Copy link
Contributor Author

The AS_TESTS axis is still unstable for WildFly testsuite timeout failures.
The test failures are three

[ERROR] testLeakingConnection(org.jboss.as.test.integration.messaging.jms.context.notclosinginjectedcontext.NotClosingInjectedContextTestCase)  Time elapsed: 6.174 s  <<< FAILURE!
[ERROR] testEjbTimeoutOnOtherNode(org.jboss.as.test.multinode.ejb.timer.database.DatabaseTimerServiceMultiNodeTestCase)  Time elapsed: 0.442 s  <<< FAILURE!
[ERROR] testCriticalAnalyzer(org.jboss.as.test.manualmode.messaging.CriticalAnalyzerTestCase)  Time elapsed: 12.207 s  <<< ERROR!

I run all of them on my local machine and they passed. As well I run the CDI ArjunaJTA tests.

The trouble with the narayana.sh is that it is not able to retain results of different test runs and then report the result of all. When WFLY testsuite fails then the ArjunaJtA/cdi tests are not run on CI.

@mayankkunwar what do you think about this PR and possibility to merge it?

@mayankkunwar
Copy link
Contributor

mayankkunwar commented Apr 12, 2021

I think we can merge your changes, as I also tested in my local, changes were working fine with tests. I was just waiting for the CI failures to pass.

@jbosstm-bot
Copy link

@ochaloup
Copy link
Contributor Author

@mayankkunwar thanks

@ochaloup ochaloup merged commit 0bbb955 into jbosstm:master Apr 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants