Skip to content

Conversation

@holly-cummins
Copy link
Contributor

Partial resolution of #44124.

As with #51059, I left the extensions/* project using an independently-managed container, since the test exercises application.properties a lot.

I'm marking this as a draft, because I had a failure I can't explain. I'm hoping @radcortez and @yrodiere might be able to help. I don't know if it's a bug, or if I'm asking the test to do something that's not supported. The jpa-postgresql project fails with

Caused by: io.quarkus.arc.InactiveBeanException: Bean is not active: SYNTHETIC bean [class=io.agroal.api.AgroalDataSource, id=83v3mgZs1bc75ou7lAXNtJCAcLA]
Reason: Datasource '<default>' was deactivated automatically because its URL is not set. To activate the datasource, set configuration property 'quarkus.datasource.jdbc.url'. Refer to https://quarkus.io/guides/datasource for guidance.

The reason seems to be related to the config override, OverrideJdbcUrlBuildTimeConfigSource, which sets a quarkus.datasource.jdbc.url, to reproduce #16123. If I disable that override by deleting src/test/resources/META-INF/services/org.eclipse.microprofile.config.spi.ConfigSource, the test all passes. With the override, I'd expect a connection failure, because quarkus.datasource.jdbc.url would disable the dev service, and there's nothing listening at the hardcoded endpoint. But instead I get an error saying the quarkus.datasource.jdbc.url property isn't set. It's like the dev service processor sees it as set, so doesn't start anything, but the AgroalRecorder sees it as not set. Maybe that's expected because of when the reading and writing of the config happens, but it makes me wonder if the test is testing what it's supposed to be testing. As far as I can tell, the override was just setting the same value as was set in application.properties already.

@quarkus-bot
Copy link

quarkus-bot bot commented Nov 17, 2025

/cc @gsmet (hibernate-orm)

@quarkus-bot

This comment has been minimized.

@radcortez
Copy link
Member

Yes, it's a trick in the source itself :)

Originally, the JDBC Extension queried the JDBC URL value at build time to either start or skip the DevService. In cases where the URL was an expansion, this would fail if the expansion is not available at build time. In the original issue, the JDBC URL was set as an expansion in Vault, only available at runtime (see what I mean that we shouldn't be querying runtime configuration at build time? :))

So I've changed that to just query for the name to be available, since we don't care about the value at all when we have to determine if we launch the DevService. To simulate the original issue, the source only sets quarkus.datasource.jdbc.url at build time, and then gives no value at runtime:

@Override
public String getValue(final String propertyName) {
if (!propertyName.equals("quarkus.datasource.jdbc.url")) {
return super.getValue(propertyName);
}
boolean isBuildTime = false;
for (ConfigSource configSource : ConfigProvider.getConfig().getConfigSources()) {
if (configSource.getName().equals("PropertiesConfigSource[source=Build system]")) {
isBuildTime = true;
break;
}
}
if (isBuildTime) {
return "jdbc:postgresql://localhost:5431/hibernate_orm_test";
}
return super.getValue(propertyName);
}

So in this case, you get a value at build time, which disables the DevService, and then when runtime starts, there is no value, which causes the message saying that the URL is not set.

Testing for this particular situation when using DevServices is mutually exclusive. We can either remove the config source and trust that our code is not querying for the actual config value (by always using ConfigUtils#isPropertyPresent or ConfigUtils#isAnyPropertyPresent), or we need the test not to use DevServices.

@holly-cummins
Copy link
Contributor Author

holly-cummins commented Nov 19, 2025

Originally, the JDBC Extension queried the JDBC URL value at build time to either start or skip the DevService. In cases where the URL was an expansion, this would fail if the expansion is not available at build time. In the original issue, the JDBC URL was set as an expansion in Vault, only available at runtime (see what I mean that we shouldn't be querying runtime configuration at build time? :))

So I've changed that to just query for the name to be available, since we don't care about the value at all when we have to determine if we launch the DevService. To simulate the original issue, the source only sets quarkus.datasource.jdbc.url at build time, and then gives no value at runtime:

Thank you for the explanation! So this would be a correct refactoring of the original test? Without this PR, that is. From

        if (isBuildTime) {
            return "${postgres.url}";
        }

        return super.getValue(propertyName);

to

        if (isBuildTime) {
            return "${arbitrary.unavailable.property}";
        }

        return super.getValue(propertyName);

@radcortez
Copy link
Member

Yes, it doesn't really matter what the expression configuration name is. We just want to ensure that the expression resolution is not available at build time and that we don't try to expand it, as that would cause an error.

@holly-cummins holly-cummins force-pushed the convert-postgresql-tests-to-devservices branch from 1243412 to caeda87 Compare November 19, 2025 17:59
@holly-cummins
Copy link
Contributor Author

Yes, it doesn't really matter what the expression configuration name is. We just want to ensure that the expression resolution is not available at build time and that we don't try to expand it, as that would cause an error.

Got it, thanks! I'll switch the property name to make it more clear it's not supposed to work. Since the other property was also used in the application.properties I thought it was supposed to be resolvable.

I've reworked the tests and I have something that passes, and I think it still exercises the same things, but I'd value a check from @radcortez. What I've done is use dev services for most of the tests in that project, and added a new test which uses a resource lifecycle manager to start postgres. I also tried @BeforeAll but of course that was way too late, and quarkus failed to start before the setup code even ran. I believe at build time the config from the resource lifecycle manager isn't present and the config source gets asked (which disables dev services), but then at runtime the resource manager operates normally. I can see in Podman Desktop a dev service doesn't get started if I run just the new test in isolation, and I can see from printlns that the config source is being invoked at build time.

@holly-cummins holly-cummins marked this pull request as ready for review November 19, 2025 18:08
@quarkus-bot

This comment has been minimized.

@holly-cummins holly-cummins marked this pull request as draft November 19, 2025 18:57
@holly-cummins holly-cummins force-pushed the convert-postgresql-tests-to-devservices branch from caeda87 to cf5c0e8 Compare November 19, 2025 21:34
@holly-cummins holly-cummins marked this pull request as ready for review November 19, 2025 21:34
@quarkus-bot
Copy link

quarkus-bot bot commented Nov 19, 2025

Status for workflow Quarkus CI

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

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

@radcortez
Copy link
Member

Excellent!


```
mvn clean install -Dtest-containers
mvn clean install -Dtest-containers -Dpostgres.url=jdbc:postgresql://...
Copy link
Member

Choose a reason for hiding this comment

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

I missed this in your other PRs... but I think it's incorrect? postgres.url is no longer mentioned in the configuration of this integration test, so I don't think setting it will do anything.

In order for this to be true, I think you'd need to keep the line quarkus.datasource.jdbc.url=${postgres.url} in application.properties. And set the default value of postgres.url to an empty string in pom.xml, so that by default the configuration will look like quarkus.datasource.jdbc.url= (no value), which means dev services will kick in.

Copy link
Member

Choose a reason for hiding this comment

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

That, or we remove this part of the README and expect people to always use dev services to run these tests. Which is perfectly fine IMO, but others might disagree -- @gsmet maybe?

Comment on lines +45 to +49
<!-- <dependency>-->
<!-- <groupId>org.junit.platform</groupId>-->
<!-- <artifactId>junit-platform-suite</artifactId>-->
<!-- <scope>test</scope>-->
<!-- </dependency>-->
Copy link
Member

Choose a reason for hiding this comment

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

Should this be removed? Or maybe we need a comment to explain why it's here and commented out?

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.

3 participants