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.

Sign up for GitHub

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

Jump to bottom

Datasource-related refactorings of tests (and some utils) #43537

Merged
merged 6 commits into from
Sep 28, 2024
Merged

Datasource-related refactorings of tests (and some utils) #43537

merged 6 commits into from
Sep 28, 2024

Conversation

yrodiere
Copy link
Member

This is a pre-requisite for #41929, but is only refactorings and addition of more tests, without changing the behavior of Quarkus.

So, I thought it would be best to merge it separately, before #41929. It won't be a waste anyway, as it clarifies some surprising behavior -- which I intend to fix in #41929 -- in the Flyway/Liquibase extensions, as well as reactive clients (see #43517).

@quarkus-bot quarkus-bot
Copy link

quarkus-bot bot commented Sep 26, 2024

/cc @gsmet (hibernate-orm)

@yrodiere
Copy link
Member Author

Also, sorry. Yes it's a lot of tests, but there are so many edge cases, it's a nightmare.

@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 26, 2024 •
edited
Loading

🎊 PR Preview 60a9780 has been successfully built and deployed to https://quarkus-pr-main-43537-preview.surge.sh/version/main/guides/

  • Images of blog posts older than 3 months are not available.
  • Newsletters older than 3 months are not available.

@quarkus-bot quarkus-bot
Copy link

quarkus-bot bot commented Sep 26, 2024 •
edited
Loading

🌘 This workflow status is outdated as a new workflow run has been triggered.

Status for workflow Quarkus CI

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

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

gsmet
gsmet approved these changes Sep 27, 2024
View reviewed changes
Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

I spotted a few very minor things. I let you decide if you want to fix them in this PR or in the follow-up. I don't mind either way.

...ent/src/main/java/io/quarkus/reactive/datasource/deployment/ReactiveDataSourceBuildUtil.java Show resolved Hide resolved
...ment/src/main/java/io/quarkus/reactive/db2/client/deployment/ReactiveDB2ClientProcessor.java Show resolved Hide resolved
...ons/liquibase/runtime/src/main/java/io/quarkus/liquibase/runtime/LiquibaseRuntimeConfig.java Outdated Show resolved Hide resolved
yrodiere
yrodiere commented Sep 27, 2024
View reviewed changes
Copy link
Member Author

@yrodiere yrodiere left a comment

Choose a reason for hiding this comment

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

Thanks for the review. I addressed all comments, let's merge once CI agrees.

@yrodiere yrodiere added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Sep 27, 2024
@yrodiere yrodiere marked this pull request as draft September 27, 2024 12:18
@yrodiere
Copy link
Member Author

I forgot about Hibernate Reactive... pushing an update to align it on Hibernate ORM.

@yrodiere
Introduce utils around datasource qualifiers
5f4ff6f 
So that we can use these more broadly in the future.
@yrodiere
Support '<default>' directly in Liquibase*Config#getConfigForDataSour…
9618549 
…ceName
@yrodiere
Refactor datasource tests related to active/inactive
85da225 
So that further changes are shorter.

This is only a refactoring and doesn't change the test expectations.
@yrodiere yrodiere marked this pull request as ready for review September 27, 2024 12:20
@quarkus-bot quarkus-bot

This comment has been minimized.

Sign in to view
@yrodiere yrodiere marked this pull request as draft September 27, 2024 13:25
@yrodiere
Refine datasource tests related to "empty config" and health checks
1076e24 
To be completely sure of what we'll change exactly in further changes.

This is only a refactoring and doesn't change existing test expectations
(though it adds some).

"Empty config" actually means "missing JDBC/reactive URL".
Testing what happens when named datasources are not defined at build
time, in particular, is beside the point.

For health checks, it's just that some configurations were not being
tested, and the next commits may affect the behavior.
@yrodiere
Handle null in DataSourceUtil#isDefault
789202f
@yrodiere
Align Hibernate Reactive datasource tests on Hibernate ORM
2633fb6
@quarkus-bot quarkus-bot bot added the area/hibernate-reactive Hibernate Reactive label Sep 27, 2024
@yrodiere yrodiere marked this pull request as ready for review September 27, 2024 14:05
@quarkus-bot quarkus-bot
Copy link

quarkus-bot bot commented Sep 27, 2024

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit 2633fb6.

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

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

@gsmet gsmet merged commit 2811eec into quarkusio:main Sep 28, 2024
63 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.16 - main milestone Sep 28, 2024
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Sep 28, 2024
@yrodiere yrodiere deleted the datasource-more-tests branch October 3, 2024 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gsmet gsmet gsmet approved these changes

Assignees
No one assigned
Labels
area/agroal area/flyway area/hibernate-orm Hibernate ORM area/hibernate-reactive Hibernate Reactive area/liquibase area/reactive-sql-clients
Projects
None yet
Milestone
3.16.0.CR1
Development

Successfully merging this pull request may close these issues.
2 participants
@yrodiere @gsmet