arquillian / arquillian-governor

Arquillian Governor
Apache License 2.0
10 stars 11 forks source link

System properties should take precedence over arquillian.xml #41

Open bartoszmajsak opened 7 years ago

bartoszmajsak commented 7 years ago

When, for example, username and password for JIRA are defined in arquillian.xml but also provided as environment variable, latter should take precedence.

Current implementation takes only those which are defined in the file.

MatousJobanek commented 7 years ago

You basically expect the completely opposite behaviour that is currently implemented in Governor - the current implementation ignores values that are set as system properties when the values are set in arq.xml and you want to do it the other way around... To be honest I would choose you approach as well, as it is also implemented in arq-core: https://github.com/arquillian/arquillian-core/blob/master/config/impl-base/src/main/java/org/jboss/arquillian/config/impl/extension/PropertiesParser.java So I'm very interested why it was done in this way - was there any good reason for it? Could you please shed some light on this @smiklosovic

bartoszmajsak commented 7 years ago

That's correct. I think that's what you should generally expect when it comes to precedence. System properties take precedence over locally defined ones (e.g. mvn).

smiklosovic commented 7 years ago

yeah why not :)

what about environment properties?

bartoszmajsak commented 7 years ago

This can be always solved in arq.xml

Maybe we should remove those additional ones and just rely on what is available from core?

bartoszmajsak commented 7 years ago

I think we should have following strategy:

I wouldn't remove "counter parts" as they are already introduced and it would be confusing for the user.

By counterparts I mean for example redmine.governor.server (used in arquillian.xml as server under configuration for redmine).

And we can always use regular substitution provided by Arquillian Core: ${property_name:default_value}.