search

quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.63k stars 2.64k forks source link

continous testing doesnt reload on changes in `.env` #41282

Closed flyinfish closed 2 months ago

flyinfish commented 3 months ago

Describe the bug

not shure if it is a "bug" - possible it might be by purpose? if there is a hint for a quick fix/improvement i might try a shot.

in continous testing application.properties is watched and test re-executed as soon as it changes. but for "local" changes in big projects where i want to include-exclude specifig test this dowes not seem the right location.

better suited for this would be .env but here changes are not immediately trigger tests and forcing it with [r] to re-run does not have any effect on i.E. quarkus.test.include-tags.

Expected behavior

changes in .env should be fully watched and quarkus.test.include-tags take effect

Actual behavior

changes in .env have only partial effect and only on force with [r]

How to Reproduce?

https://github.com/flyinfish/quarkus--continuous-testing-dotenv

Output of uname -a or ver

Linux C70C7391 5.15.153.1-microsoft-standard-WSL2 #1 SMP Fri Mar 29 23:14:13 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux

Output of java -version

openjdk 21.0.3 2024-04-16 LTS OpenJDK Runtime Environment Zulu21.34+20-SA (build 21.0.3+9-LTS) OpenJDK 64-Bit Server VM Zulu21.34+20-SA (build 21.0.3+9-LTS, mixed mode, sharing)

Quarkus version or git rev

3.12.1

Build tool (ie. output of mvnw --version or gradlew --version)

Apache Maven 3.9.6 (bc0240f3c744dd6b6ec2920b3cd08dcc295161ae) Maven home: /home/u125015/.m2/wrapper/dists/apache-maven-3.9.6-bin/3311e1d4/apache-maven-3.9.6 Java version: 21.0.3, vendor: Azul Systems, Inc., runtime: /usr/lib/jvm/mobi-azuljdk21 Default locale: en, platform encoding: UTF-8 OS name: "linux", version: "5.15.153.1-microsoft-standard-wsl2", arch: "amd64", family: "unix"

Additional information

No response

gsmet commented 3 months ago

I created https://github.com/quarkusio/quarkus/pull/41317 .

gsmet commented 2 months ago

Just a heads up that I made a second attempt at improving things in my PR. It's less of a hack and also we fully monitor .env for dev mode now.

Let's hope CI will agree with me and we will be able to merge it for 3.13 :).

flyinfish commented 2 months ago

thank you @gsmet for your change, unfortunately CI does not seem to agree

gsmet commented 2 months ago

Yeah. So it agreed on the first version but something is apparently incorrect with the new improved version.

I haven't forgotten about it, I will have a closer look later this week as I'm deep into class loader leaks these days. I'll try to get it in for 3.13.

flyinfish commented 2 months ago

speaking of classloaders - i left a comment on pr. might not be big news to you but saves you hopefully a sec or two?

gsmet commented 2 months ago

@flyinfish in which PR did you comment? I don't see it?

In any case, I pushed an update that will hopefully make CI happy, and it was indeed CL-related. I initially pushed a CL for the config source we discover via service loader but given I disabled all that we can use the test CL and things appear to be working fine as we are now loading the config files with the proper CL.

Let's hope CI will agree :).

flyinfish commented 2 months ago

jaarrgh - i did not submit the review.

obsolete anyhow as green now. i saw that getMinimalConfig() did not pick up application.properties because of classloader.