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

karate 1.4.1- Dynamic property ( set in runner) not overridden from maven command #2474

Open
rc2201 opened this issue Jan 4, 2024 · 9 comments
Assignees
Labels
Milestone

Comments

@rc2201
Copy link

rc2201 commented Jan 4, 2024

Attached is the repo to reproduce the issue.

Details:-

  1. dynamic properties karate.username and karate.password are defined in the config with default value.
  2. Have dummy values setup in the runner file i.e. ExamplesTest.java.
  3. karate version is set to 1.4.1 in attached repo.

Execute below maven command-
mvn test -Dtest=ExamplesTest -Dkarate.username=UsernamefromCmd -Dkarate.password=PasswordfromCmd
check the payload for the API call in summary report. Username and password value is being picked from runner.

  1. Change the karate version to 1.4.0 in pom file and run the command from#3 again.

Payload is using correct value of username and password i.e. values passed in the maven command.

Attaching the screenshot showing payload for 1.4 and 1.4.1.

Dynamic Property Override 1 4
Dynamic Property Override 1 4 1
karatePropertyOverride.zip

@ptrthomas
Copy link
Member

probably related to #2444 - contributions welcome

@dvargas46
Copy link
Contributor

this appears to be from this change.

if the preference should be the JVM system properties over the ones configured by the builder, then a fix could be to switch back that part of the change in the runner.

@ptrthomas
Copy link
Member

@dvargas46 I'm unable to figure this out by reading the code, maybe needs some debugging

@dvargas46
Copy link
Contributor

@ptrthomas just tried debugging it and this does appear to be the issue from what I could tell.

going from

systemProperties.putAll(new HashMap(System.getProperties()));

to this

Map temp = new HashMap(System.getProperties());
temp.putAll(systemProperties); // make sure user-specified takes precedence
systemProperties = temp;

reverses the previous behavior. I.e., now the systemProperties set on the java builder is overwriting any common system properties defined on the jvm (using -D), instead of how it was before.

I think most would expect the jvm props to take precedence, at least from my experience. Let me know and I can put out a small PR to bring back the old behavior here.

@ptrthomas
Copy link
Member

@dvargas46 thanks, yes please do proceed with a PR

dvargas46 added a commit to dvargas46/karate that referenced this issue Jan 16, 2024
ptrthomas added a commit that referenced this issue Jan 18, 2024
fix system properties issue on runner #2474
@ptrthomas ptrthomas added this to the 1.5.0 milestone Jan 20, 2024
@ptrthomas
Copy link
Member

karate 1.5.0.RC3 including this fix is now available

@rc2201
Copy link
Author

rc2201 commented Feb 2, 2024

I tested this out against 1.5.0RC3 and still seeing the issue.

Changes made to POM in earlier attached project- Updated the karate version and group Id.

<karate.version>1.5.0.RC3</karate.version>
<groupId>io.karatelabs</groupId>

Ran the same command provided earlier and still seeing username and password being picked from runner and not being overridden.

@ptrthomas ptrthomas removed the fixed label Feb 3, 2024
@dvargas46
Copy link
Contributor

Yes, it looks like some changes were made after my PR that brought back the issue.

@ptrthomas , I can create another PR that should fix the core issue here while retaining the new changes made. Or would you prefer to wait until after 1.5.0 is released?

@ptrthomas
Copy link
Member

@dvargas46 thanks, apologies since I may have broken it with some other changes to the runner / suite part.

please go ahead with another PR, there will be at least one more RC release of 1.5.0 before final

dvargas46 added a commit to dvargas46/karate that referenced this issue Feb 16, 2024
ptrthomas added a commit that referenced this issue Feb 18, 2024
ptrthomas added a commit that referenced this issue Feb 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants