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

Generating Jackson serializers for concrete classes only #44080

Merged
merged 1 commit into from
Oct 25, 2024

Conversation

mariofusco
Copy link
Contributor

Fixes #44069

/cc @gsmet

This comment has been minimized.

@mariofusco
Copy link
Contributor Author

mariofusco commented Oct 24, 2024

@gsmet I don't see any reason why all those tests should fail with JDK21 and I cannot reproduce any of those problems locally. Can you please simply relaunch that job? Or do you have any better explanation?

@mariofusco
Copy link
Contributor Author

@gsmet sorry, ignore my latest comment, for some reason I cannot reproduce the problem from maven CLI but I can when launching those tests from inside the IDE. I need a bit more time to figure out what's happening.

Copy link

quarkus-bot bot commented Oct 25, 2024

Status for workflow Quarkus CI

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

✅ 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 Windows

📦 extensions/resteasy-reactive/rest-client/deployment

io.quarkus.rest.client.reactive.stork.StorkResponseTimeLoadBalancerTest.shouldUseFasterService - History

  • expected: "hello, Alice" but was: "hello, I'm a slow server" - org.opentest4j.AssertionFailedError
org.opentest4j.AssertionFailedError: 

expected: "hello, Alice"
 but was: "hello, I'm a slow server"
	at io.quarkus.rest.client.reactive.stork.StorkResponseTimeLoadBalancerTest.shouldUseFasterService(StorkResponseTimeLoadBalancerTest.java:58)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)
	at io.quarkus.test.QuarkusUnitTest.runExtensionMethod(QuarkusUnitTest.java:513)
	at io.quarkus.test.QuarkusUnitTest.interceptTestMethod(QuarkusUnitTest.java:427)

@gsmet
Copy link
Member

gsmet commented Oct 25, 2024

@mariofusco out of curiosity, what was the issue?

@mariofusco
Copy link
Contributor Author

@gsmet it should be ok now. The problem is that it was uselessly trying to generate serializers also for not concrete classes (abstract classes or interfaces) and moreover trying to register them into the Jackson ObjectMapper caused an error internally to Jackson during serialization.

@gsmet
Copy link
Member

gsmet commented Oct 25, 2024

That part I understood :). I was more wondering what caused a specific Java 21 failure :)

@gsmet gsmet merged commit f617828 into quarkusio:main Oct 25, 2024
32 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.17 - main milestone Oct 25, 2024
@mariofusco
Copy link
Contributor Author

That part I understood :). I was more wondering what caused a specific Java 21 failure :)

Sorry I misunderstood your question :)

Actually it was a real bug in my yesterday's commit. It is true that I don't want to generate serializers for abstract classes, but with what I did yesterday it was skipping in the generated serializers for a concrete class the fields eventually inherited from its abstract parent. To be honest I have no clue why this failed only on Java 21.

@gsmet
Copy link
Member

gsmet commented Oct 25, 2024

OK, thanks!

@gsmet gsmet modified the milestones: 3.17 - main, 3.16.1 Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Recently added ReflectionFreeSerializationTest is very unstable
2 participants