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

Properly read the value of testcontainers.reuse.enable #30140

Merged
merged 1 commit into from
Jan 3, 2023

Conversation

geoand
Copy link
Contributor

@geoand geoand commented Jan 3, 2023

As mentioned by @LaunchPairs in the analysis of #30059, the use of TestcontainersConfiguration#getUserProperty results in using only environment variables as the source for reading the testcontainers.reuse.enable.
We should instead also read the value of ~/.testcontainers.properties in order to ensure we write back the proper value when restoring

Fixes: #30059

As mentioned by @LaunchPairs in the analysis of quarkusio#30059,
the use of TestcontainersConfiguration#getUserProperty
results in using only environment variables as the source
for reading the testcontainers.reuse.enable.
We should instead also read the value of ~/.testcontainers.properties
in order to ensure we write back the proper value when restoring

Fixes: quarkusio#30059
@@ -88,7 +88,7 @@ public Result get() {
.loadClass("org.testcontainers.utility.TestcontainersConfiguration");
Object configurationInstance = configurationClass.getMethod("getInstance").invoke(null);
String oldReusePropertyValue = (String) configurationClass
.getMethod("getUserProperty", String.class, String.class)
.getMethod("getEnvVarOrUserProperty", String.class, String.class)
Copy link
Contributor Author

@geoand geoand Jan 3, 2023

Choose a reason for hiding this comment

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

There is also a case to be made for using getEnvVarOrProperty here, but let's be cautious and only broaden the scope a little instead of all the way

@geoand geoand added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Jan 3, 2023
@quarkus-bot
Copy link

quarkus-bot bot commented Jan 3, 2023

Failing Jobs - Building 0ff1d5d

Status Name Step Failures Logs Raw logs
✔️ JVM Tests - JDK 11
✔️ JVM Tests - JDK 17
JVM Tests - JDK 17 MacOS M1 Set up runner ⚠️ Check → Logs Raw logs
✔️ JVM Tests - JDK 18

@geoand geoand merged commit 6a31744 into quarkusio:main Jan 3, 2023
@quarkus-bot quarkus-bot bot added this to the 2.16 - main milestone Jan 3, 2023
@geoand geoand deleted the #30059 branch January 3, 2023 16:25
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Jan 3, 2023
@gsmet gsmet modified the milestones: 2.16 - main, 2.15.2.Final Jan 3, 2023
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.

testcontainers.reuse.enable always set to false
3 participants