Skip to content

Conversation

gsmet
Copy link
Member

@gsmet gsmet commented May 7, 2025

Fixes #47744
Fixes #47473
Related to #47563

@Postremus it was really handy to have the test case all ready, thanks! :).

This comment has been minimized.

@geoand
Copy link
Contributor

geoand commented May 7, 2025

CI failures seem related

@gsmet
Copy link
Member Author

gsmet commented May 7, 2025

@geoand yeah, I just pushed a second commit. My first one apparently discovered a bug that was here all along.

Comment on lines +16 to +17
this.internalName = name.replace('.', '/');
this.binaryName = name.replace('/', '.');
Copy link
Member Author

Choose a reason for hiding this comment

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

This change is to make things more consistent with GeneratedClassBuildItem and have the same methods available.

They are generated as application classes so we should consider them as
such.
This is probably a very old bug, discovered by my previous fix.
@gsmet gsmet force-pushed the fix-application-class branch from 8439180 to 396ab2f Compare May 7, 2025 17:44
Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

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

Thanks!

This comment has been minimized.

@geoand geoand added the triage/waiting-for-ci Ready to merge when CI successfully finishes label May 8, 2025
@gsmet
Copy link
Member Author

gsmet commented May 8, 2025

@geoand yeah this issue already happened a few times. Using a fixed port for the TLS reload test is probably not a good idea but not sure how we could do better.

Copy link

quarkus-bot bot commented May 8, 2025

Status for workflow Quarkus CI

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

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

@geoand geoand merged commit 25cc0e0 into quarkusio:main May 8, 2025
53 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.23 - main milestone May 8, 2025
@quarkus-bot quarkus-bot bot added kind/bugfix and removed triage/waiting-for-ci Ready to merge when CI successfully finishes labels May 8, 2025
@gsmet
Copy link
Member Author

gsmet commented May 8, 2025

@holly-cummins while the previous behavior was indeed buggy, do you know why these problems started to pop, everywhere in the code base?

@holly-cummins
Copy link
Contributor

holly-cummins commented May 8, 2025

@holly-cummins while the previous behavior was indeed buggy, do you know why these problems started to pop, everywhere in the code base?

I don't. :(

Here are some possible perturbations triggered by my changes that could be surfacing these:

  • Tests run in a different order [not a bug, but different, so can expose bugs]
  • Classes loaded in a different order. I'd need to check to confirm, but it could be that we used to load the Resource, and then the Test, and now we load the Test, and then the Resource.
  • Different TCCL during some stage of the process [usually a bug but there's no one 'correct' TCCL to aim for]
  • For this particular issue, it used to be that the test would have been partially executed in the system classloader before being re-executed in the quarkus classloader. I can imagine that maybe would change something?
  • Config badness, such as config arriving too late, or being attached to the wrong classloader, so some behaviour-changing config isn't right [usually a bug]
  • Extension processors all run at the beginning, instead of just before the test (where there's profiles, so I don't think that change would apply here) [as designed, but needs other code changes to not break stuff]

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.

IllegalAccessError with REST Resources outside of the application module Quarkus (REST) behaves differently when having a parent POM
3 participants