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

Pass quarkus args to dev mode gradle task #29250

Merged
merged 1 commit into from
Jan 31, 2023

Conversation

jacobdotcosta
Copy link
Contributor

@jacobdotcosta jacobdotcosta commented Nov 14, 2022

Tried to build a test case for this fix but wasn't able to do so as dev mode doesn't end until you either press CTRL+C or q (quit) on the console .

Tested the fix using the jacobdotcosta/quarkus-issue-28443 project.

Closes #28443

@quarkus-bot
Copy link

quarkus-bot bot commented Nov 14, 2022

Thanks for your pull request!

The title of your pull request does not follow our editorial rules. Could you have a look?

  • title should preferably start with an uppercase character (if it makes sense!)
  • title should not start with chore/docs/feat/fix/refactor but be a proper sentence

This message is automatically generated by a bot.

@quarkus-bot quarkus-bot bot added area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/gradle Gradle labels Nov 14, 2022
@jacobdotcosta jacobdotcosta changed the title fix: pass quarkus args to dev mode gradle task Pass quarkus args to dev mode gradle task Nov 14, 2022
Copy link
Member

@glefloch glefloch 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 this @jacobdotcosta. I will try to push a tests I worked on.

@aloubyansky
Copy link
Member

@jacobdotcosta
Copy link
Contributor Author

@jacobdotcosta Have you seen how dev mode is tested in https://github.com/quarkusio/quarkus/tree/main/integration-tests/gradle/src/test/java/io/quarkus/gradle/devmode?

No, I didn't encounter those tests when I looked for them 🤦🏽 . I'll take a look at them now. Thank you @aloubyansky .

@jacobdotcosta
Copy link
Contributor Author

I've implemented a test case for this but it's failing and I need your help with it. Let's hope I can explain myself correctly.

I wasn't sure of a way to check the main method information result as there is no access to the logs in the current io.quarkus.gradle.devmode.QuarkusDevGradleTestBase class implementation. The way I did it was by using 2 static variables in the org.acme.EntryPoint class which is implementing the main method.

The problem I'm having is that the result of parsing the -Dquarkus.args="param1=1 param2=2" in the Java integration tests is different from calling the dev mode from the command line, let me explain.

When I launch the test with the -Dquarkus.args="param1=1 param2=2" parameter I expect that it will be translated to an array of 2 String as it happens when I call the dev mode from the command line. The problem is that the testing dev mode isn't making this parsing so the test is failing.

The test output is the following.

...
[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running io.quarkus.gradle.devmode.BasicJavaMainApplicationModuleDevModeTest
$ /...../quarkus/integration-tests/gradle/./gradlew -D=maven.repo.local=/home/xxx/.m2/repository -Dorg.gradle.console=plain --stacktrace --info clean quarkusDev -Dquarkus.args="param1=1 param2=2"
getHttpResponse: [param1=1 param2=2]
getHttpResponse: 1
BELOW IS THE CAPTURED LOGGING OF THE FAILED GRADLE TEST PROJECT BUILD
...
Press [h] for more options>
Tests paused
Press [r] to resume testing, [h] for more options>
Press [r] to resume testing, [o] Toggle test output, [h] for more options>
args.length: 1
args: [param1=1 param2=2]
__  ____  __  _____   ___  __ ____  ______ 
 --/ __ \/ / / / _ | / _ \/ //_/ / / / __/ 
 -/ /_/ / /_/ / __ |/ , _/ ,< / /_/ /\ \   
--\___\_\____/_/ |_/_/|_/_/|_|\____/___/   
2022-11-24 14:05:10,001 INFO  [io.quarkus] (Quarkus Main Thread) code-with-quarkus unspecified on JVM (powered by Quarkus 999-SNAPSHOT) started in 1.988s. Listening on: http://localhost:8080
2022-11-24 14:05:10,018 INFO  [io.quarkus] (Quarkus Main Thread) Profile dev activated. Live Coding activated.
2022-11-24 14:05:10,019 INFO  [io.quarkus] (Quarkus Main Thread) Installed features: [cdi, resteasy, smallrye-context-propagation, vertx]

[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 10.242 s <<< FAILURE! - in io.quarkus.gradle.devmode.BasicJavaMainApplicationModuleDevModeTest
[ERROR] io.quarkus.gradle.devmode.BasicJavaMainApplicationModuleDevModeTest.main  Time elapsed: 9.959 s  <<< FAILURE!
java.lang.AssertionError: 

Expecting actual:
  "1"
to contain:
  "2" 

But if I execute the /...../quarkus/integration-tests/gradle/./gradlew -D=maven.repo.local=/home/ajc102/.m2/repository -Dorg.gradle.console=plain --stacktrace --info clean quarkusDev -Dquarkus.args="param1=1 param2=2" from the built project (by temporary removing the line that deletes the temporary projectDir FileUtils.deleteQuietly(projectDir);), I get the expected result.

Press [h] for more options>
Tests paused
Press [r] to resume testing, [h] for more options>
Press [r] to resume testing, [o] Toggle test output, [h] for more options>
args.length: 2
args: [param1=1, param2=2]
__  ____  __  _____   ___  __ ____  ______ 
 --/ __ \/ / / / _ | / _ \/ //_/ / / / __/ 
 -/ /_/ / /_/ / __ |/ , _/ ,< / /_/ /\ \   
--\___\_\____/_/ |_/_/|_/_/|_|\____/___/   
2022-11-24 13:53:32,811 INFO  [io.quarkus] (Quarkus Main Thread) code-with-quarkus unspecified on JVM (powered by Quarkus 999-SNAPSHOT) started in 1.469s. Listening on: http://localhost:8080
2022-11-24 13:53:32,825 INFO  [io.quarkus] (Quarkus Main Thread) Profile dev activated. Live Coding activated.
2022-11-24 13:53:32,825 INFO  [io.quarkus] (Quarkus Main Thread) Installed features: [cdi, resteasy, smallrye-context-propagation, vertx]

Could you give me a hand with this @glefloch @aloubyansky

@aloubyansky
Copy link
Member

@jacobdotcosta you can see how the log is printed in QuarkusDevGradleTestBase. You could try reading the command-output.log or add some changes to expose the log to the test, for example as a post test assertion.

As to the parsing, you could try checking the value of quarkus.args in QuarkusDev, it probably somehow gets quotes and that's an issue.

@quarkus-bot

This comment has been minimized.

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.

Just a note that this will need a squash before merging.

@quarkus-bot

This comment has been minimized.

@gsmet
Copy link
Member

gsmet commented Dec 2, 2022

I rebased and force pushed. There's a good chance the test failure is not related but let's get another CI run.

@jacobdotcosta
Copy link
Contributor Author

Thank you @gsmet , I was testing doing the squash as I never did it before. 😨

The test error is something that I haven't been able to find yet the reason. If I comment the delete temporary project delete and launch the same gradlew command it works. But the one executed inside the unit test parses the arguments wrong as 1 argument only.

@quarkus-bot

This comment has been minimized.

@jacobdotcosta
Copy link
Contributor Author

The problem is in the way java.lang.ProcessBuilder treats the arguments, on class io.quarkus.gradle.QuarkusGradleWrapperTestBase.

When starting Quarkus dev mode from the command line the -Dquarkus.args= must have double quotes to group all the arguments, e.g. -Dquarkus.args="param1=1 param2=2". Nevertheless the same string passed to java.lang.ProcessBuilder is treated as 1 whole string parameter and isn't split in an array when it arrives to the main method. For java.lang.ProcessBuilder to work correctly the -Dquarkus.args parameter must be passed as 1 command entry but without the double quotes (e.g. -Dquarkus.args=param1=1 param2=2).

I'll remove the double quotes from the test's buildArguments() but I'm not sure if this might be better fixed on the io.quarkus.gradle.QuarkusGradleWrapperTestBase class so the build arguments on the tests are similar to those executed on the command line.

@jacobdotcosta jacobdotcosta requested review from gsmet and removed request for aloubyansky December 13, 2022 07:45
@quarkus-bot

This comment has been minimized.

@jacobdotcosta
Copy link
Contributor Author

The MacOS tests have failed. I can see a devtools/gradle/gradlew: No such file or directory but I'm not sure what is causing this.

...
2023-01-04T15:29:22.9663660Z Starting machine "podman-machine-default"
2023-01-04T15:29:23.4807730Z Waiting for VM ...
2023-01-04T15:29:38.8691100Z Mounting volume... /Volumes/actions-files/githubactions:/Volumes/actions-files/githubactions
2023-01-04T15:29:39.3572700Z Error: exit status 255
2023-01-04T15:29:39.3588410Z ##[error]Process completed with exit code 125.
2023-01-04T15:29:39.3763220Z ##[group]Run devtools/gradle/gradlew --stop && rm -rf devtools/gradle/gradle-extension-plugin/build/tmp
2023-01-04T15:29:39.3763550Z �[36;1mdevtools/gradle/gradlew --stop && rm -rf devtools/gradle/gradle-extension-plugin/build/tmp�[0m
2023-01-04T15:29:39.3779080Z shell: /bin/bash --noprofile --norc -e -o pipefail {0}
2023-01-04T15:29:39.3779250Z env:
2023-01-04T15:29:39.3779370Z   LANG: en_US.UTF-8
2023-01-04T15:29:39.3779550Z   COMMON_MAVEN_ARGS: -e -B --settings .github/mvn-settings.xml --fail-at-end
2023-01-04T15:29:39.3779970Z   NATIVE_TEST_MAVEN_ARGS: -Dtest-containers -Dstart-containers -Dquarkus.native.native-image-xmx=5g -Dnative -Dnative.surefire.skip -Dformat.skip -Dno-descriptor-tests install -DskipDocs
...
2023-01-04T15:29:39.3781250Z ##[endgroup]
2023-01-04T15:29:39.4106020Z /Volumes/actions-files/githubactions/actions-runner/_work/_temp/fad6c767-635b-422d-9804-50b6c61f4a52.sh: line 1: devtools/gradle/gradlew: No such file or directory
2023-01-04T15:29:39.4109370Z ##[error]Process completed with exit code 1.

@aloubyansky
Copy link
Member

@jacobdotcosta please squash your commits into one.

@aloubyansky
Copy link
Member

@gsmet could you please give it another look?

@gsmet gsmet merged commit 63bfcba into quarkusio:main Jan 31, 2023
@quarkus-bot quarkus-bot bot added this to the 2.17 - main milestone Jan 31, 2023
@gsmet
Copy link
Member

gsmet commented Jan 31, 2023

Thanks a lot and sorry for the delay!

@gsmet
Copy link
Member

gsmet commented Jan 31, 2023

Apparently the test is failing in main. I will revert it for now and open a new PR tomorrow.

@jacobdotcosta
Copy link
Contributor Author

I'll take a look.

@gsmet
Copy link
Member

gsmet commented Feb 1, 2023

@jacobdotcosta OK. Let me know if you need anything from us.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/gradle Gradle kind/bugfix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

quarkus.args not work with gradle and quarkus 2.13.0
4 participants