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

Reinstate changes to use JBoss serializer instead of xstream #40906

Merged
merged 1 commit into from
Jul 2, 2024

Conversation

holly-cummins
Copy link
Contributor

@holly-cummins holly-cummins commented May 30, 2024

This reinstates #40601. See discussion in #40880. This PR reintroduces @dmlloyd's lovely #40601, now that JUnit has been updated to a version which is compatible with #40601, 5.10.3.

It also includes a fix for a functional issue in the change that we couldn't see when it was blocked by JUnit problems (and earlier, by a lack of test coverage).

@holly-cummins holly-cummins marked this pull request as draft May 30, 2024 17:04
@quarkus-bot quarkus-bot bot added area/dependencies Pull requests that update a dependency file area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/testing labels May 30, 2024
Copy link

quarkus-bot bot commented May 30, 2024

Thanks for your pull request!

The title of your pull request does not follow our editorial rules. Could you have a look?

  • title should not contain an issue number (use Fix #1234 in the description instead)

This message is automatically generated by a bot.

@holly-cummins holly-cummins changed the title Reinstate #40601 to use JBoss Serializer (and update JUnit) Reinstate changes to use JBoss serializer instead of xstream (and update JUnit) May 30, 2024

This comment has been minimized.

@quarkus-bot quarkus-bot bot added area/arc Issue related to ARC (dependency injection) area/gradle Gradle area/platform Issues related to definition and interaction with Quarkus Platform labels May 30, 2024
@dmlloyd
Copy link
Member

dmlloyd commented Jun 27, 2024

@holly-cummins
Copy link
Contributor Author

https://github.com/junit-team/junit5/releases/tag/r5.10.3 🎉

🎉 🎉

Will update to that level in this PR and see how CI goes (did we get the backport?)

@dmlloyd
Copy link
Member

dmlloyd commented Jun 27, 2024

Yeah it should include the backport we need.

@holly-cummins holly-cummins marked this pull request as ready for review June 27, 2024 17:50
@holly-cummins
Copy link
Contributor Author

PR amended to use JUnit 5.10.3.

Copy link
Member

@dmlloyd dmlloyd left a comment

Choose a reason for hiding this comment

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

LGTM, obviously :)

@gsmet gsmet added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Jun 27, 2024
@geoand
Copy link
Contributor

geoand commented Jun 28, 2024

The test failure is definitely related:

2024-06-27T22:31:00.7017409Z Press [e] to edit command line args (currently ''), [r] to re-run, [o] Toggle test output, [h] for more options>
2024-06-27T22:31:00.7019039Z 2024-06-27 22:31:00,573 ERROR [io.qua.test] (Test runner thread) ==================== TEST REPORT #1 ====================
2024-06-27T22:31:00.7020830Z 2024-06-27 22:31:00,574 ERROR [io.qua.test] (Test runner thread) Test TemplatedQuarkusTest#[@io.quarkus.test.junit.QuarkusTest()] failed 
2024-06-27T22:31:00.7022050Z : java.lang.ExceptionInInitializerError
2024-06-27T22:31:00.7022979Z 	at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499)
2024-06-27T22:31:00.7024208Z 	at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:480)
2024-06-27T22:31:00.7025819Z 	at org.jboss.marshalling.reflect.SerializableClass.invokeConstructorNoException(SerializableClass.java:404)
2024-06-27T22:31:00.7027534Z 	at org.jboss.marshalling.reflect.SerializableClass.callNonInitConstructor(SerializableClass.java:372)
2024-06-27T22:31:00.7029013Z 	at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:309)
2024-06-27T22:31:00.7030301Z 	at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:132)
2024-06-27T22:31:00.7031660Z 	at org.jboss.marshalling.cloner.SerializingCloner.cloneFields(SerializingCloner.java:452)
2024-06-27T22:31:00.7033155Z 	at org.jboss.marshalling.cloner.SerializingCloner.initSerializableClone(SerializingCloner.java:372)
2024-06-27T22:31:00.7034614Z 	at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:315)
2024-06-27T22:31:00.7036045Z 	at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:132)
2024-06-27T22:31:00.7037450Z 	at org.jboss.marshalling.cloner.SerializingCloner$StepObjectInput.doReadObject(SerializingCloner.java:930)
2024-06-27T22:31:00.7038955Z 	at org.jboss.marshalling.AbstractObjectInput.readObject(AbstractObjectInput.java:41)
2024-06-27T22:31:00.7040462Z 	at org.jboss.marshalling.MarshallerObjectInputStream.readObjectOverride(MarshallerObjectInputStream.java:53)
2024-06-27T22:31:00.7042912Z 	at java.base/java.io.ObjectInputStream.readObject(ObjectInputStream.java:500)
2024-06-27T22:31:00.7044111Z 	at java.base/java.io.ObjectInputStream.readObject(ObjectInputStream.java:467)
2024-06-27T22:31:00.7045326Z 	at java.base/java.util.ArrayList.readObject(ArrayList.java:899)
2024-06-27T22:31:00.7046513Z 	at org.jboss.marshalling.reflect.JDKSpecific$SerMethods.callReadObject(JDKSpecific.java:171)
2024-06-27T22:31:00.7047969Z 	at org.jboss.marshalling.reflect.SerializableClass.callReadObject(SerializableClass.java:252)
2024-06-27T22:31:00.7049590Z 	at org.jboss.marshalling.cloner.SerializingCloner.initSerializableClone(SerializingCloner.java:366)
2024-06-27T22:31:00.7051145Z 	at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:315)
2024-06-27T22:31:00.7052507Z 	at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:132)
2024-06-27T22:31:00.7054069Z 	at org.jboss.marshalling.cloner.SerializingCloner.cloneFields(SerializingCloner.java:452)
2024-06-27T22:31:00.7055734Z 	at org.jboss.marshalling.cloner.SerializingCloner.initSerializableClone(SerializingCloner.java:372)
2024-06-27T22:31:00.7057225Z 	at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:315)
2024-06-27T22:31:00.7058675Z 	at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:132)
2024-06-27T22:31:00.7060297Z 	at org.jboss.marshalling.cloner.SerializingCloner$StepObjectInput.doReadObject(SerializingCloner.java:930)
2024-06-27T22:31:00.7061824Z 	at org.jboss.marshalling.AbstractObjectInput.readObject(AbstractObjectInput.java:41)
2024-06-27T22:31:00.7063427Z 	at org.jboss.marshalling.MarshallerObjectInputStream.readObjectOverride(MarshallerObjectInputStream.java:53)
2024-06-27T22:31:00.7064929Z 	at java.base/java.io.ObjectInputStream.readObject(ObjectInputStream.java:500)
2024-06-27T22:31:00.7066255Z 	at java.base/java.io.ObjectInputStream.readObject(ObjectInputStream.java:467)
2024-06-27T22:31:00.7067387Z 	at java.base/java.util.ArrayList.readObject(ArrayList.java:899)
2024-06-27T22:31:00.7068559Z 	at org.jboss.marshalling.reflect.JDKSpecific$SerMethods.callReadObject(JDKSpecific.java:171)
2024-06-27T22:31:00.7070045Z 	at org.jboss.marshalling.reflect.SerializableClass.callReadObject(SerializableClass.java:252)
2024-06-27T22:31:00.7071659Z 	at org.jboss.marshalling.cloner.SerializingCloner.initSerializableClone(SerializingCloner.java:366)
2024-06-27T22:31:00.7073166Z 	at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:315)
2024-06-27T22:31:00.7074498Z 	at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:132)
2024-06-27T22:31:00.7076010Z 	at org.jboss.marshalling.cloner.SerializingCloner.cloneFields(SerializingCloner.java:452)
2024-06-27T22:31:00.7077533Z 	at org.jboss.marshalling.cloner.SerializingCloner.initSerializableClone(SerializingCloner.java:372)
2024-06-27T22:31:00.7079009Z 	at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:315)
2024-06-27T22:31:00.7080360Z 	at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:132)
2024-06-27T22:31:00.7081769Z 	at org.jboss.marshalling.cloner.SerializingCloner.cloneFields(SerializingCloner.java:452)
2024-06-27T22:31:00.7083329Z 	at org.jboss.marshalling.cloner.SerializingCloner.initSerializableClone(SerializingCloner.java:372)
2024-06-27T22:31:00.7084856Z 	at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:315)
2024-06-27T22:31:00.7086329Z 	at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:132)
2024-06-27T22:31:00.7087714Z 	at org.jboss.marshalling.cloner.SerializingCloner.cloneFields(SerializingCloner.java:452)
2024-06-27T22:31:00.7089280Z 	at org.jboss.marshalling.cloner.SerializingCloner.initSerializableClone(SerializingCloner.java:372)
2024-06-27T22:31:00.7090832Z 	at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:315)
2024-06-27T22:31:00.7092327Z 	at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:132)
2024-06-27T22:31:00.7093882Z 	at org.jboss.marshalling.cloner.SerializingCloner$StepObjectInput.doReadObject(SerializingCloner.java:930)
2024-06-27T22:31:00.7095617Z 	at org.jboss.marshalling.AbstractObjectInput.readObject(AbstractObjectInput.java:41)
2024-06-27T22:31:00.7097291Z 	at org.jboss.marshalling.MarshallerObjectInputStream.readObjectOverride(MarshallerObjectInputStream.java:53)
2024-06-27T22:31:00.7098833Z 	at java.base/java.io.ObjectInputStream.readObject(ObjectInputStream.java:500)
2024-06-27T22:31:00.7100050Z 	at java.base/java.io.ObjectInputStream.readObject(ObjectInputStream.java:467)
2024-06-27T22:31:00.7101144Z 	at java.base/java.util.ArrayList.readObject(ArrayList.java:899)
2024-06-27T22:31:00.7102344Z 	at org.jboss.marshalling.reflect.JDKSpecific$SerMethods.callReadObject(JDKSpecific.java:171)
2024-06-27T22:31:00.7103975Z 	at org.jboss.marshalling.reflect.SerializableClass.callReadObject(SerializableClass.java:252)
2024-06-27T22:31:00.7105627Z 	at org.jboss.marshalling.cloner.SerializingCloner.initSerializableClone(SerializingCloner.java:366)
2024-06-27T22:31:00.7107059Z 	at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:315)
2024-06-27T22:31:00.7108415Z 	at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:132)
2024-06-27T22:31:00.7109774Z 	at org.jboss.marshalling.cloner.SerializingCloner.cloneFields(SerializingCloner.java:452)
2024-06-27T22:31:00.7111251Z 	at org.jboss.marshalling.cloner.SerializingCloner.initSerializableClone(SerializingCloner.java:372)
2024-06-27T22:31:00.7112756Z 	at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:315)
2024-06-27T22:31:00.7114105Z 	at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:132)
2024-06-27T22:31:00.7115795Z 	at org.jboss.marshalling.cloner.SerializingCloner.cloneFields(SerializingCloner.java:452)
2024-06-27T22:31:00.7116808Z 	at org.jboss.marshalling.cloner.SerializingCloner.initSerializableClone(SerializingCloner.java:372)
2024-06-27T22:31:00.7117659Z 	at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:315)
2024-06-27T22:31:00.7118421Z 	at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:132)
2024-06-27T22:31:00.7119301Z 	at org.jboss.marshalling.cloner.SerializingCloner$StepObjectInput.doReadObject(SerializingCloner.java:930)
2024-06-27T22:31:00.7120174Z 	at org.jboss.marshalling.AbstractObjectInput.readObject(AbstractObjectInput.java:41)
2024-06-27T22:31:00.7121079Z 	at org.jboss.marshalling.MarshallerObjectInputStream.readObjectOverride(MarshallerObjectInputStream.java:53)
2024-06-27T22:31:00.7121939Z 	at java.base/java.io.ObjectInputStream.readObject(ObjectInputStream.java:500)
2024-06-27T22:31:00.7122625Z 	at java.base/java.io.ObjectInputStream.readObject(ObjectInputStream.java:467)
2024-06-27T22:31:00.7123238Z 	at java.base/java.util.ArrayList.readObject(ArrayList.java:899)
2024-06-27T22:31:00.7123926Z 	at org.jboss.marshalling.reflect.JDKSpecific$SerMethods.callReadObject(JDKSpecific.java:171)
2024-06-27T22:31:00.7124765Z 	at org.jboss.marshalling.reflect.SerializableClass.callReadObject(SerializableClass.java:252)
2024-06-27T22:31:00.7125882Z 	at org.jboss.marshalling.cloner.SerializingCloner.initSerializableClone(SerializingCloner.java:366)
2024-06-27T22:31:00.7126723Z 	at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:315)
2024-06-27T22:31:00.7127479Z 	at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:132)
2024-06-27T22:31:00.7128298Z 	at org.jboss.marshalling.cloner.SerializingCloner.cloneFields(SerializingCloner.java:452)
2024-06-27T22:31:00.7129176Z 	at org.jboss.marshalling.cloner.SerializingCloner.initSerializableClone(SerializingCloner.java:372)
2024-06-27T22:31:00.7130016Z 	at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:315)
2024-06-27T22:31:00.7130878Z 	at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:132)
2024-06-27T22:31:00.7131670Z 	at org.jboss.marshalling.cloner.SerializingCloner.cloneFields(SerializingCloner.java:452)
2024-06-27T22:31:00.7132544Z 	at org.jboss.marshalling.cloner.SerializingCloner.initSerializableClone(SerializingCloner.java:372)
2024-06-27T22:31:00.7133473Z 	at org.jboss.marshalling.cloner.SerializingCloner.initSerializableClone(SerializingCloner.java:340)
2024-06-27T22:31:00.7134314Z 	at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:315)
2024-06-27T22:31:00.7135058Z 	at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:132)
2024-06-27T22:31:00.7136132Z 	at org.jboss.marshalling.cloner.SerializingCloner.cloneFields(SerializingCloner.java:452)
2024-06-27T22:31:00.7137094Z 	at org.jboss.marshalling.cloner.SerializingCloner.initSerializableClone(SerializingCloner.java:372)
2024-06-27T22:31:00.7138091Z 	at org.jboss.marshalling.cloner.SerializingCloner.initSerializableClone(SerializingCloner.java:340)
2024-06-27T22:31:00.7138927Z 	at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:315)
2024-06-27T22:31:00.7139665Z 	at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:132)
2024-06-27T22:31:00.7140494Z 	at org.jboss.marshalling.cloner.SerializingCloner.cloneFields(SerializingCloner.java:452)
2024-06-27T22:31:00.7141410Z 	at org.jboss.marshalling.cloner.SerializingCloner.initSerializableClone(SerializingCloner.java:372)
2024-06-27T22:31:00.7142321Z 	at org.jboss.marshalling.cloner.SerializingCloner.initSerializableClone(SerializingCloner.java:340)
2024-06-27T22:31:00.7143162Z 	at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:315)
2024-06-27T22:31:00.7143902Z 	at org.jboss.marshalling.cloner.SerializingCloner.clone(SerializingCloner.java:132)
2024-06-27T22:31:00.7144725Z 	at io.quarkus.test.junit.internal.NewSerializingDeepClone.clone(NewSerializingDeepClone.java:92)
2024-06-27T22:31:00.7145795Z 	at io.quarkus.test.junit.QuarkusTestExtension.runExtensionMethod(QuarkusTestExtension.java:965)
2024-06-27T22:31:00.7146668Z 	at io.quarkus.test.junit.QuarkusTestExtension.runExtensionMethod(QuarkusTestExtension.java:905)
2024-06-27T22:31:00.7147610Z 	at io.quarkus.test.junit.QuarkusTestExtension.interceptTestTemplateMethod(QuarkusTestExtension.java:865)
2024-06-27T22:31:00.7148462Z 	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183)
2024-06-27T22:31:00.7149195Z 	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
2024-06-27T22:31:00.7149941Z 	at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:179)
2024-06-27T22:31:00.7150655Z 	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
2024-06-27T22:31:00.7151418Z 	at java.base/java.util.stream.Streams$StreamBuilderImpl.forEachRemaining(Streams.java:411)
2024-06-27T22:31:00.7152197Z 	at java.base/java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:762)
2024-06-27T22:31:00.7152943Z 	at java.base/java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:276)
2024-06-27T22:31:00.7153738Z 	at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1625)
2024-06-27T22:31:00.7154506Z 	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
2024-06-27T22:31:00.7155423Z 	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
2024-06-27T22:31:00.7156217Z 	at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150)
2024-06-27T22:31:00.7157016Z 	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173)
2024-06-27T22:31:00.7157794Z 	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
2024-06-27T22:31:00.7158531Z 	at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596)
2024-06-27T22:31:00.7159208Z 	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
2024-06-27T22:31:00.7159726Z 	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
2024-06-27T22:31:00.7160257Z Caused by: java.util.NoSuchElementException: No value present
2024-06-27T22:31:00.7160740Z 	at java.base/java.util.Optional.get(Optional.java:143)
2024-06-27T22:31:00.7161386Z 	at io.quarkus.deployment.dev.testing.TestConsoleHandler.<clinit>(TestConsoleHandler.java:38)
2024-06-27T22:31:00.7161942Z 	... 117 more
2024-06-27T22:31:00.7162066Z 
2024-06-27T22:31:00.7162610Z 2024-06-27 22:31:00,582 ERROR [io.qua.test] (Test runner thread) >>>>>>>>>>>>>>>>>>>> Summary: <<<<<<<<<<<<<<<<<<<<
2024-06-27T22:31:00.7163302Z TemplatedQuarkusTest#[@io.quarkus.test.junit.QuarkusTest()] null
2024-06-27T22:31:00.7164100Z 2024-06-27 22:31:00,585 ERROR [io.qua.test] (Test runner thread) >>>>>>>>>>>>>>>>>>>> 1 TEST FAILED <<<<<<<<<<<<<<<<<<<<
2024-06-27T22:31:00.7164947Z 1 test failed (2 passing, 0 skipped), 3 tests were run in 2656ms. Tests completed at 22:31:00.
2024-06-27T22:31:08.6068071Z [ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 32.81 s <<< FAILURE! -- in io.quarkus.it.extension.it.TestTemplateDevModeIT
2024-06-27T22:31:08.6069789Z [ERROR] io.quarkus.it.extension.it.TestTemplateDevModeIT.testThatTheTestsPassed -- Time elapsed: 32.81 s <<< FAILURE!
2024-06-27T22:31:08.6070735Z org.opentest4j.AssertionFailedError: expected: <0> but was: <1>
2024-06-27T22:31:08.6071439Z 	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
2024-06-27T22:31:08.6072865Z 	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
2024-06-27T22:31:08.6074205Z 	at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
2024-06-27T22:31:08.6075342Z 	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:166)
2024-06-27T22:31:08.6076036Z 	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:161)
2024-06-27T22:31:08.6076689Z 	at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:632)
2024-06-27T22:31:08.6077505Z 	at io.quarkus.it.extension.it.TestTemplateDevModeIT.testThatTheTestsPassed(TestTemplateDevModeIT.java:59)
2024-06-27T22:31:08.6078281Z 	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
2024-06-27T22:31:08.6078807Z 	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
2024-06-27T22:31:08.6079312Z 	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

@holly-cummins
Copy link
Contributor Author

holly-cummins commented Jun 28, 2024

Yeah, that test failure is the one we'd expect if @dmlloyd's fix hadn't been backported.

It's hard to tell if it's been backported or not. Confusing!
junit-team/junit5#3837 is the 5.10.x PR, which is still open. (junit-team/junit5#3820 is the 5.11.x PR, which is merged.)

But ... if I check https://github.com/junit-team/junit5/commits/releases/5.10.x/, I can see 3820 in there, which would be consistent with them just doing the backport themselves.
image

So then the question is ... why is 5.10.3 + these changes regressing that test? :(

@holly-cummins
Copy link
Contributor Author

I've checked #40773, and the stack trace is a bit different. Without 5.10.3, the stack trace is:

2024-05-22 11:53:10,847 ERROR [io.qua.test] (Test runner thread) ==================== TEST REPORT #1 ====================
2024-05-22 11:53:10,847 ERROR [io.qua.test] (Test runner thread) Test TemplatedQuarkusTest#[@io.quarkus.test.junit.QuarkusTest()] failed 
: java.lang.NullPointerException: Cannot invoke "org.junit.platform.engine.UniqueId.toString()" because the return value of "org.junit.platform.launcher.TestIdentifier.access$800(org.junit.platform.launcher.TestIdentifier)" is null

So this is definitely a regression, and one that seems related to either 5.10.3 or #40601, but more investigation will be needed. We can apply 5.10.3 without #40601, so I'll try that first, just for diagnostics.

@geoand
Copy link
Contributor

geoand commented Jun 28, 2024

We can apply 5.10.3

We actually already have it in main. Dependabot did a PR that I merged

@holly-cummins
Copy link
Contributor Author

holly-cummins commented Jun 28, 2024

We can apply 5.10.3

We actually already have it in main. Dependabot did a PR that I merged

Ah, I looked for the dependabot PR but didn't think to look at the closed ones. I should have remembered how fast you are. :)

OK, I'll confirm whether this affects Pact as well (seems likely), confirm I can reproduce locally, and see what I can see.

@geoand
Copy link
Contributor

geoand commented Jun 28, 2024

I should have remembered how fast you are. :)

Or basically that I am 2 hours ahead in my TZ :P

@holly-cummins
Copy link
Contributor Author

I don't think the failure in the CI can be anything to do with these changes. All tests on one machine failed with:

Error:  io.quarkus.it.main.ServletITCase.null
Error:    Run 1: ServletITCase » IllegalState Unable to locate the artifact metadata file created that must be created by Quarkus in order to run integration tests. Make sure this test is run after 'mvn package'. 

@geoand
Copy link
Contributor

geoand commented Jun 30, 2024

Indeed that looks unrelated!

You can try running the failing job again just to be on the safe side

@holly-cummins
Copy link
Contributor Author

Indeed that looks unrelated!

You can try running the failing job again just to be on the safe side

I think that needs higher privileges than I have: https://github.com/quarkusio/quarkus/actions/runs/9724907070/job/26841777161?pr=40906 is the failing job, but I could only re-run it by doing a rebase and re-running everything.

@geoand
Copy link
Contributor

geoand commented Jun 30, 2024

I kicked it off!

@holly-cummins
Copy link
Contributor Author

holly-cummins commented Jun 30, 2024

Hmm, same failure. I'll rebase to trigger a new build, in case the problem was a glitch in the process that farms work out to the matrix sub-builds.

@holly-cummins holly-cummins force-pushed the reinstate-40601 branch 2 times, most recently from a08c827 to 6b63320 Compare July 1, 2024 09:32
@gsmet
Copy link
Member

gsmet commented Jul 1, 2024

Sorry to be the bearer of bad news, but I would be surprised if it was a glitch. Never seen than before and I wouldn't be surprised if it was an actual issue.

The IT main thing is a bit of a weird beast (quite large and mixing quite a lot of extensions/patterns).

@holly-cummins
Copy link
Contributor Author

Sorry to be the bearer of bad news, but I would be surprised if it was a glitch. Never seen than before and I wouldn't be surprised if it was an actual issue.

What makes me suspect it's a glitch is that it appears to be a packaging issue (the error message claims a file is missing on disk). If a code-level issue in this change affected discoverability of the file, it should affect quite a few sub-builds, not just the one. The failure is across the board in one 'child', but the identical child with virtual threads enabled passes fine, as do all the other native children.

This change clones fewer classes than #40601 (which clones a lot more classes than HEAD), so I can imagine a mechanism where the resource ends up in the wrong classloader, and the integration test are doing getResource() in the other classloader and can't find it. But I've checked the code and the code which looks for the file is looking on disk:

    public static Properties readQuarkusArtifactProperties(ExtensionContext context) {
        Path buildOutputDirectory = determineBuildOutputDirectory(context);
        Path artifactProperties = buildOutputDirectory.resolve("quarkus-artifact.properties");
        if (!Files.exists(artifactProperties)) {

The IT main thing is a bit of a weird beast (quite large and mixing quite a lot of extensions/patterns).

I'm trying to reproduce locally, but without luck so far; I've given podman machine 4gb and am running with

MAVEN_OPTS=-Xmx6g mvn -pl integration-tests/main -Dtest-containers -Dstart-containers -Dquarkus.native.native-image-xmx=8g -Dnative -Dnative.surefire.skip -Dno-descriptor-tests -DskipDocs -Dquarkus.native.container-build=true verify

and my local build is OOMing:

[ERROR] Failed to execute goal io.quarkus:quarkus-maven-plugin:999-SNAPSHOT:build (default) on project quarkus-integration-test-main: Failed to build quarkus application: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
[ERROR] 	[error]: Build step io.quarkus.deployment.pkg.steps.NativeImageBuildStep#build threw an exception: io.quarkus.deployment.pkg.steps.NativeImageBuildStep$ImageGenerationFailureException: Image generation failed. Exit code was 137 which indicates an out of memory error. The most likely cause is Docker not being given enough memory. Also consider increasing the Xmx value for native image generation by setting the "quarkus.native.native-image-xmx" property
[ERROR] 	at io.quarkus.deployment.pkg.steps.NativeImageBuildStep.imageGenerationFailed(NativeImageBuildStep.java:462)

This comment has been minimized.

@gsmet
Copy link
Member

gsmet commented Jul 1, 2024

I think we are experiencing this very issue elsewhere: #41561 .

@gsmet
Copy link
Member

gsmet commented Jul 1, 2024

I think it's related to the Develocity cache extension that is being developed by the Gradle team and @aloubyansky (cc @jprinet). I will have a look.

@holly-cummins
Copy link
Contributor Author

I think it's related to the Develocity cache extension that is being developed by the Gradle team and @aloubyansky (cc @jprinet). I will have a look.

Vindicaaaatioooooon! :)

While trying to figure out what in my change could have caused this, I ended up doing the follow-on refactoring I wanted to do, to reduce duplication between the checks in the serialization class and QuarkusTestExtension. I have those pushed to this PR now (and it looks like the latest build has dodged the cache, and will execute cleanly). However, I should squash the changes before we merge this (and then hope the follow-on build also dodges the cache).

I kept support for configuring what classes get cloned, with the Quarkus-set default being java.*. This was put in two or three years ago with #9677 as part of efforts to make Pact work. We should definitely get rid of that config when we move to loading tests with the intended classloader, and I wonder if we could get rid of it earlier. Cloning classes in java.* seems unnecessary. I'll investigate in a follow-on PR.

@holly-cummins
Copy link
Contributor Author

Ok, clean build:

image

My amendments on top of the amended version above are f0f899ff905 and I've kicked off a squashed build.

…#40749"

This reverts commit fc3988b.

It has some additional changes which re-revert part of quarkusio#40601, to reintroduce removed guards to avoid cloning things like Quarkus runtime classes. Otherwise we get test failures.

This comment has been minimized.

@gsmet
Copy link
Member

gsmet commented Jul 1, 2024

I think this can be merged as soon as CI has passed. I'm not worried about the flaky tests.

Copy link

quarkus-bot bot commented Jul 1, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit eef5138.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.


Flaky tests - Develocity

⚙️ JVM Tests - JDK 17

📦 integration-tests/reactive-messaging-kafka

io.quarkus.it.kafka.KafkaConnectorTest.testFruits - History

  • Assertion condition defined as a Lambda expression in io.quarkus.it.kafka.KafkaConnectorTest expected: <6> but was: <5> within 10 seconds. - org.awaitility.core.ConditionTimeoutException
org.awaitility.core.ConditionTimeoutException: Assertion condition defined as a Lambda expression in io.quarkus.it.kafka.KafkaConnectorTest expected: <6> but was: <5> within 10 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:119)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:31)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1006)
	at org.awaitility.core.ConditionFactory.untilAsserted(ConditionFactory.java:790)
	at io.quarkus.it.kafka.KafkaConnectorTest.testFruits(KafkaConnectorTest.java:63)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)

⚙️ JVM Tests - JDK 21

📦 extensions/smallrye-reactive-messaging-kafka/deployment

io.quarkus.smallrye.reactivemessaging.kafka.deployment.dev.KafkaDevServicesDevModeTestCase.sseStream - History

  • Assertion condition defined as a Lambda expression in io.quarkus.smallrye.reactivemessaging.kafka.deployment.dev.KafkaDevServicesDevModeTestCase Expecting size of: [] to be greater than or equal to 2 but was 0 within 10 seconds. - org.awaitility.core.ConditionTimeoutException
org.awaitility.core.ConditionTimeoutException: 
Assertion condition defined as a Lambda expression in io.quarkus.smallrye.reactivemessaging.kafka.deployment.dev.KafkaDevServicesDevModeTestCase 
Expecting size of:
  []
to be greater than or equal to 2 but was 0 within 10 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:119)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:31)

⚙️ JVM Tests - JDK 17 Windows

📦 integration-tests/rest-client

io.quarkus.it.rest.client.wronghost.ExternalWrongHostUsingVerifyHostTestCase.restClient - History

  • Read timed out - java.net.SocketTimeoutException
java.net.SocketTimeoutException: Read timed out
	at java.base/sun.nio.ch.NioSocketImpl.timedRead(NioSocketImpl.java:288)
	at java.base/sun.nio.ch.NioSocketImpl.implRead(NioSocketImpl.java:314)
	at java.base/sun.nio.ch.NioSocketImpl.read(NioSocketImpl.java:355)
	at java.base/sun.nio.ch.NioSocketImpl$1.read(NioSocketImpl.java:808)
	at java.base/java.net.Socket$SocketInputStream.read(Socket.java:966)
	at org.apache.http.impl.io.AbstractSessionInputBuffer.fillBuffer(AbstractSessionInputBuffer.java:161)
	at org.apache.http.impl.io.SocketInputBuffer.fillBuffer(SocketInputBuffer.java:82)

📦 integration-tests/rest-client-reactive

io.quarkus.it.rest.client.wronghost.ExternalWrongHostTestCase.restClientRejected - History

  • 1 expectation failed. Response body doesn't match expectation. Expected: a string containing "SSLHandshakeException" Actual: ConnectTimeoutException - java.lang.AssertionError
java.lang.AssertionError: 
1 expectation failed.
Response body doesn't match expectation.
Expected: a string containing "SSLHandshakeException"
  Actual: ConnectTimeoutException

	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:77)

@geoand geoand merged commit 6ecbfc5 into quarkusio:main Jul 2, 2024
52 checks passed
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Jul 2, 2024
@quarkus-bot quarkus-bot bot added this to the 3.13 - main milestone Jul 2, 2024
@holly-cummins holly-cummins deleted the reinstate-40601 branch July 2, 2024 08:29
@gsmet
Copy link
Member

gsmet commented Jul 2, 2024

Awesome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/arc Issue related to ARC (dependency injection) area/dependencies Pull requests that update a dependency file area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/gradle Gradle area/platform Issues related to definition and interaction with Quarkus Platform area/testing triage/flaky-test
Projects
Development

Successfully merging this pull request may close these issues.

4 participants