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

Enable test which now works because of serialization changes #40749

Merged
merged 2 commits into from
May 22, 2024

Conversation

holly-cummins
Copy link
Contributor

#35124 added several tests which could not pass, because of our test classloading issues. It's taken me a while (cough) to get on top of the code changes to make them pass, but @dmlloyd has fixed one category of them (and also quarkiverse/quarkus-pact#73) with #40601.

Because the test has been disabled for a while, it rotted slightly and got broken by some dev ui changes. TheDevUIJsonRPCTest was seeing a non-default quarkus.http.non-application-root-path, so things failed. I can’t see where the problem value is coming from; perhaps some sticky config changes from a previous test? Cross-talk between tests is bad, but I didn’t want to get sucked into a config rabbit hole, so I just set my application to have the same config as the intruding config and moved on.

I also confirmed that #27821 and #22611 are still problems (and patched up their tests to fail for the right reasons again, so they will hopefully pass when the epic test classloading rewrite ETCR (#34681) is done.

dmlloyd added a commit to dmlloyd/quarkus that referenced this pull request May 21, 2024
Relates to quarkusio#40601, quarkusio#40749, quarkusio#38991. No use waiting for Dependabot. Fixes problem where an NPE can occur in some tests.

This comment has been minimized.

@holly-cummins
Copy link
Contributor Author

holly-cummins commented May 21, 2024

The failing tests are in my new test, so it looks like I need to be a bit more successful disabling it on native.

Edit: Now fixed.

@holly-cummins holly-cummins marked this pull request as draft May 21, 2024 18:26
@holly-cummins holly-cummins marked this pull request as ready for review May 21, 2024 19:13
Copy link

quarkus-bot bot commented May 21, 2024

Status for workflow Quarkus CI

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

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

@@ -1,193 +0,0 @@
# Log settings
# log level in lower case for testing
quarkus.log.level=info
Copy link
Member

Choose a reason for hiding this comment

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

Why is this file removed? It was useless?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I initially wrote these tests, I'd copied the files from another test. It had a whole bunch of settings that we didn't actually want in this test, such as setting the non-application-root to 1234. (Although, as mentioned above, even without the settings file, the non-application root is still 1234, which I haven't managed to explain.)

I left the file, so we had change history, but removed all of the settings since we didn't need any of them, as far as I know.

@gsmet gsmet merged commit d9b98cd into quarkusio:main May 22, 2024
19 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.12 - main milestone May 22, 2024
holly-cummins added a commit to holly-cummins/quarkus that referenced this pull request May 29, 2024
holly-cummins added a commit to holly-cummins/quarkus that referenced this pull request May 29, 2024
holly-cummins added a commit to holly-cummins/quarkus that referenced this pull request May 30, 2024
holly-cummins added a commit to holly-cummins/quarkus that referenced this pull request May 30, 2024
holly-cummins added a commit to holly-cummins/quarkus that referenced this pull request May 30, 2024
holly-cummins added a commit to holly-cummins/quarkus that referenced this pull request May 30, 2024
holly-cummins added a commit to holly-cummins/quarkus that referenced this pull request Jun 27, 2024
holly-cummins added a commit to holly-cummins/quarkus that referenced this pull request Jun 28, 2024
…#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.
holly-cummins added a commit to holly-cummins/quarkus that referenced this pull request Jun 29, 2024
…#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.
holly-cummins added a commit to holly-cummins/quarkus that referenced this pull request Jun 30, 2024
…#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.
holly-cummins added a commit to holly-cummins/quarkus that referenced this pull request Jul 1, 2024
…#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.
holly-cummins added a commit to holly-cummins/quarkus that referenced this pull request Jul 1, 2024
…#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.
holly-cummins added a commit to holly-cummins/quarkus that referenced this pull request Jul 31, 2024
holly-cummins added a commit to holly-cummins/quarkus that referenced this pull request Jul 31, 2024
…#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.
danielsoro pushed a commit to danielsoro/quarkus that referenced this pull request Sep 20, 2024
danielsoro pushed a commit to danielsoro/quarkus that referenced this pull request Sep 20, 2024
…#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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants