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

Fixing gradleCmd to allow args to have spaces similarly to fix on PR 921 #2029

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from

Conversation

keeganjm-bsc
Copy link

@keeganjm-bsc keeganjm-bsc commented Jun 13, 2023

  • All tests passed. If this feature is not already covered by the tests, I added new tests.
  • All static analysis checks passed.
  • This pull request is on the dev branch.
  • I used gofmt for formatting the code before submitting the pull request.

This PR is related to #921 and is needed to pass in multiple tags with space. For example:
-Dkarate.options="--tags=@smoke --tags=~@regression"

This command fails when being passed as the gradle command using jfrog cli like so
'jfrog rt gradle clean assemble myTest -Dkarate.options="--tags=@smoke --tags=~@regression"'

Currently the gradleCmd is not taking the complete value of -Dkarate.options and is interpreting the white space in the command as a break in the cmd line argument for -Dkarate.options and is then looking at --tags as a commandline argument. Here is an example of how it is appearing in the logs.

'jfrog rt gradle clean assemble myTest -Dkarate.options=--tags=@smoke --tags=~@regression'

Note the space after '@smoke'. This is what appears to be causing us issues. As the error coming into our logs is saying --tags is not a command line argument.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 13, 2023

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@keeganjm-bsc
Copy link
Author

I have read the CLA Document and I hereby sign the CLA

@yahavi yahavi added the safe to test Approve running integration tests on a pull request label Jun 14, 2023
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Jun 14, 2023
@keeganjm-bsc
Copy link
Author

@yahavi. I am curious if there is anything I can do/what is needed to get this merged into dev/production, and how long does this typically take?

@keeganjm-bsc
Copy link
Author

@yahavi Any update on this would be much appreciated. Thanks in advance!

@yahavi yahavi added the safe to test Approve running integration tests on a pull request label Jun 17, 2023
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Jun 17, 2023
@yahavi
Copy link
Member

yahavi commented Jun 17, 2023

Hello @keeganjm-bsc and thanks for your contribution!
As you may know, this code does not compile and therefore is not mergeable. It requires some work in the jfrog-cli-core.
Would you like to add an additional PR to the jfrog-cli-core that completes this behavior change?

@keeganjm-bsc
Copy link
Author

@yahavi Created a PR for this in cli-core project. Here is that PR jfrog/jfrog-cli-core#830. Let me know if there is anything else I can do to move this forward. Thanks!

@Addison-CA
Copy link

Hello @yahavi, could you please assist further. Hoping that you can find some time to review Keegan's change and they are accepted and can be merged. Many thanks in advance for your support. Cheers, Chris

@keeganjm-bsc
Copy link
Author

@yahavi I have the PR for these changes to compile in the cli-core project here jfrog/jfrog-cli-core#848. All the tests are passing and are waiting for review to be merged. Please review that PR and once that PR is merged trigger a build on this PR so we can get these changes merged. Thanks

@yahavi
Copy link
Member

yahavi commented Jun 30, 2023

@keeganjm-bsc I tests your changes in my environment and I'm not convinced that this resolves the issue.
I built the JFrog CLI on this PR code and jfrog/jfrog-cli-core#848.

This command passed successfully:

./jf gradle clean aP -Dkarate.options='asdasdas'

But this one doesn't, with or without your changes the results are the same:

./jf gradle clean aP -Dkarate.options='asd asdas'

I'm getting this error:

Task 'bbb' not found in root project 'gradle-example-ci-server'.

Did you thoroughly test this modification prior to submitting the pull request? Is there something I may be overlooking?
Thanks!

@keeganjm-bsc
Copy link
Author

@yahavi as I am not familiar with how the Jfrog project is setup and how I can test my changes with other dependencies I am basing my changes off of how mvn args with spaces was implemented. Please refer me to any docs on how I can test this change in my local environment as that is not entirely clear from any documentation I have seen provided from Jfrog.

@keeganjm-bsc
Copy link
Author

@keeganjm-bsc I tests your changes in my environment and I'm not convinced that this resolves the issue. I built the JFrog CLI on this PR code and jfrog/jfrog-cli-core#848.

This command passed successfully:

./jf gradle clean aP -Dkarate.options='asdasdas'

But this one doesn't, with or without your changes the results are the same:

./jf gradle clean aP -Dkarate.options='asd asdas'

I'm getting this error:

Task 'bbb' not found in root project 'gradle-example-ci-server'.

Did you thoroughly test this modification prior to submitting the pull request? Is there something I may be overlooking? Thanks!

Also, this error 'task 'bbb' not found in root project 'gradle-example-ci-server' is questionable. Looking at your gradle command where are you passing in task bbb?

@yahavi
Copy link
Member

yahavi commented Jun 30, 2023

Also, this error 'task 'bbb' not found in root project 'gradle-example-ci-server' is questionable. Looking at your gradle command where are you passing in task bbb?

The task 'bbb' does not exist. The "karate.options" is simply a property. Nevertheless, the command -Dkarate.options='asdasdas' works, whereas -Dkarate.options='asd asdas' does not. Shouldn't this be the issue that this pull request is intended to resolve?

I would like to confirm once more whether this change has resolved your issue. Please inform me if I have overlooked anything in this regard.

@keeganjm-bsc
Copy link
Author

This change should resolve my issue i am trying to do some further testing on my end. @yahavi how are you pulling my changes from the cli-core project into this project? I have tried updating the go.mod to point to my repo but it is not successfully pulling those changes at the moment

@keeganjm-bsc
Copy link
Author

@yahavi I was able to point to my version of the jfrog cli core. Still running into issues in testing it. Can you show me how you are testing this in your local env or provide an example?

@yahavi
Copy link
Member

yahavi commented Jul 13, 2023

@keeganjm-bsc thanks again for contributing to the JFrog CLI repository.

Can you show me how you are testing this in your local env or provide an example?

Sure.
You can use this sample project as your test project: https://github.com/jfrog/project-examples/tree/master/gradle-examples/gradle-example-ci-server
To check this code run the following commands:

# Configure the Gradle project and the Artifactory repositories
./jf gradlec

# Run a simple Gradle command
./jf gradle clean aP -Dkarate.options='aaa bbb'

If the second command fails with "Task 'bbb' not found in root project" that means that your fix did not resolve this issue.

@keeganjm-bsc
Copy link
Author

@keeganjm-bsc thanks again for contributing to the JFrog CLI repository.

Can you show me how you are testing this in your local env or provide an example?

Sure. You can use this sample project as your test project: https://github.com/jfrog/project-examples/tree/master/gradle-examples/gradle-example-ci-server To check this code run the following commands:

# Configure the Gradle project and the Artifactory repositories
./jf gradlec

# Run a simple Gradle command
./jf gradle clean aP -Dkarate.options='aaa bbb'

If the second command fails with "Task 'bbb' not found in root project" that means that your fix did not resolve this issue.

@yahavi thanks for this. I will be working on this in our upcoming sprint. I was part of a meeting with Jfrog last week, specifically Dominic. He was made aware of these PRs and we will be taking up this work in the coming sprint to address this issue. I will checkout the sample repo and test the changes locally. If I run into any issues would we be able to set-up some time to discuss this in the next week? Let me know. Thanks again for the dialogue.

Keegan

@keeganjm-bsc
Copy link
Author

@yahavi Thanks for the back and forth on this. I have a couple of questions that I am hoping you can help answer.

  1. When setting up this project locally and configuring it(credentials. URL), what is the best way to do this? Is it with docker and using just admin and password for credentials to configure the gradle project to run against the artifactory instance?
  2. Once getting artifactory set-up for the projects to run against how do I integrate the changes from the cli project and the cli-core project to test those changes out?

I have jfrog running with docker but when trying to configure the example project mentioned above to run on this instance it is not able to successfully build the project. What is the recommended way to do this project with docker running locally?

@keeganjm-bsc
Copy link
Author

keeganjm-bsc commented Aug 6, 2023

@yahavi I have pushed up some additional changes in the other repo(cli-core) related to gradle_opts and have the gradle example project configured in my local. I have updated the dependency in go.mod where we can define where we are pulling our cli-core dependency and pointed it to my local repo with the changes made.

Now my next question is how do I test these changes with the example project provided, that is, how do I point my clone of the example project to use the the changes made, and pulled into my local version of the jfrog-cli project? I do not see anywhere in the gradle example project where it defines this and how we can integrate our own version to test with this project?

@Addison-CA
Copy link

Addison-CA commented Aug 6, 2023 via email

@yahavi yahavi added the safe to test Approve running integration tests on a pull request label Aug 14, 2023
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Aug 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants