Passing multiple values for one parameter name (@Option)

412 views
Skip to first unread message

Denis Karpenko

unread,
Feb 10, 2016, 10:24:15 AM2/10/16
to gradle-dev
Hello


It would be great to have the possibility of including multiple tests for execution.

./gradlew test —tests SomeTest.foo —tests SomeTest.bar

At the moment test task uses @Option mechanism for passing external parameters. And there is a possibility to pass lists as Option parameters

But there are a few places in the code which don’t allow setting List values.

A quick check has shown me that it’s possible to pass multiple parameters via command line to test task with following changes in the code.
Please take a look at the draft version of possible PR: https://github.com/dkarpenko/gradle/pull/2

You can try it on test project by executing following command

There is only one failed test:

org.gradle.internal.filewatch.DefaultFileSystemChangeWaiterTest > waits until there is a quiet period - append and close FAILED
    org.spockframework.runtime.ConditionNotSatisfiedError at DefaultFileSystemChangeWaiterTest.groovy:167


I was initially checking my changes against REL_2.6 and there were two more relevant failures:

org.gradle.api.internal.tasks.options.OptionNotationParserFactorySpec > fails on creating parser for unsupported FAILED
    org.spockframework.runtime.WrongExceptionThrownError at OptionNotationParserFactorySpec.groovy:48

org.gradle.api.internal.tasks.options.OptionReaderTest > fail when parameter cannot be converted from the command-line FAILED
    org.spockframework.runtime.WrongExceptionThrownError at OptionReaderTest.groovy:95


Could you please help me with following questions:
  • Are there any good reasons for not allowing lists to be passed to tasks?
  • What is the convenient way of passing multiple param values to tasks in gradle? (gradle -x task1 -x task2)
  • Does anybody have any concerns on adding support for that?

Links


Best regards,
Denis Karpenko

Mark Vieira

unread,
Feb 10, 2016, 12:54:15 PM2/10/16
to gradl...@googlegroups.com
  • Are there any good reasons for not allowing lists to be passed to tasks?
Depending on the nature of the option it often doesn't make sense to accept multiple values. 
  • What is the convenient way of passing multiple param values to tasks in gradle? (gradle -x task1 -x task2)
These are command line arguments, not task options and would therefore require a completely separate solution. There are other examples where this would make sense too (-I for passing init scripts) but again would have to be handled case by case. 
  • Does anybody have any concerns on adding support for that?
We shouldn't change the behavior of `@Option` because that would mean refactoring all tasks that use it to handle being passed a collection. We would instead want to either 1) introduce a new annotation or 2) add a parameter to `@Option` to explicitly declare that the parameter is a collection type.

There a separate issue here, which is that `@Option` is not a public API (although folks are definitely using it). This means that solving the use case above (allowing multiple test filter specs) and extending `@Option` to allow multivalues are really orthogonal problems, although the solution might be the same.

The questions we need to answer are:
  1. Is supporting multiple test filter specs via the CLI something we want?
  2. If so, should we enhance `@Option` to support this or solve some other way?
  3. If we decide to enhance `@Option` or not, is this something we should consider making public?

--
You received this message because you are subscribed to the Google Groups "gradle-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to gradle-dev+...@googlegroups.com.
To post to this group, send email to gradl...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/gradle-dev/40db9ae7-5a6f-41b8-adeb-df20f2f67e52%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
--
Mark Vieira
Principal Engineer
Gradle, Inc.

Denis Karpenko

unread,
Feb 10, 2016, 4:35:06 PM2/10/16
to gradle-dev
Hi Mark

We shouldn't change the behavior of `@Option` because that would mean refactoring all tasks that use it to handle being passed a collection. We would instead want to either 1) introduce a new annotation or 2) add a parameter to `@Option` to explicitly declare that the parameter is a collection type.

I don't actually think that the behaviour of @Option will be changed: in my example user (developer) need to specify explicitly that he want to receive a list of parameters. So, existing tasks which are using @Options will still accept only one argument and shouldn't be refactored.

@Option(option = "tests", description = "Sets test class or method name to be included, '*' is supported.") 
- public Test setTestNameIncludePattern(String testNamePattern) { 
+ public Test setTestNameIncludePattern(List<String> testNamePattern) {

It's definitely not the case for all tasks.


  1. Is supporting multiple test filter specs via the CLI something we want?
I believe that ability to select/filter tests for execution is something which shouldn't require any changes in build script or writing custom code. One example is re-running only failed tests when preparing your changes to be committed.

2. If so, should we enhance `@Option` to support this or solve some other way?
If I understand correctly '@Option'  already supports multiple arguments. But, due to some reason, there some asserts were added which were not allowing to use this feature. 


Best regards,
Denis Karpenko


 

 
 

Mark Vieira

unread,
Feb 10, 2016, 4:54:09 PM2/10/16
to gradl...@googlegroups.com
If I understand correctly '@Option'  already supports multiple arguments. But, due to some reason, there some asserts were added which were not allowing to use this feature. 

I believe you are conflating command line options (ex: gradle --info) with task options (ex. gradle dependencies --configuration compile). These are two completely separate things with different implementations. The @Option annotation only applies to the latter. Currently @Option does not support collection types.

My point being that we can potentially support the mutliple test filter use case without making changes to @Option. One argument to extending @Option to support collection types is that it might be useful for other things. That said, it's not a public API so that shouldn't be a motivation in this case. It's also not a trivial thing to implement. Handling multiple values means using a delimiter of some kind. We can't use a space because of ambiguity reasons. In the case of `gradle test --tests foo bar` is 'bar' a test filter or a task? Picking an arbitrary delimiter is problematic too because it might be a valid character for the parameter depending on the task implementation. 

In this case I think it's better to just have individual tasks implement this rather than have a general solution. In the case of test filtering we could just use `|` or something similar as a delimiter and parse the string in the task impl.

--
You received this message because you are subscribed to the Google Groups "gradle-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to gradle-dev+...@googlegroups.com.
To post to this group, send email to gradl...@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

Adam Murdoch

unread,
Feb 10, 2016, 7:13:43 PM2/10/16
to gradl...@googlegroups.com
On Thu, Feb 11, 2016 at 4:54 AM, Mark Vieira <ma...@gradle.com> wrote:
  • Are there any good reasons for not allowing lists to be passed to tasks?
Depending on the nature of the option it often doesn't make sense to accept multiple values. 
  • What is the convenient way of passing multiple param values to tasks in gradle? (gradle -x task1 -x task2)
These are command line arguments, not task options and would therefore require a completely separate solution. There are other examples where this would make sense too (-I for passing init scripts) but again would have to be handled case by case. 
  • Does anybody have any concerns on adding support for that?
We shouldn't change the behavior of `@Option` because that would mean refactoring all tasks that use it to handle being passed a collection. We would instead want to either 1) introduce a new annotation or 2) add a parameter to `@Option` to explicitly declare that the parameter is a collection type.

It is declared already - the signature of the setter method that `@Option` is attached to will tell us whether the property is single or multi-valued.

 

There a separate issue here, which is that `@Option` is not a public API (although folks are definitely using it). This means that solving the use case above (allowing multiple test filter specs) and extending `@Option` to allow multivalues are really orthogonal problems, although the solution might be the same.

The questions we need to answer are:
  1. Is supporting multiple test filter specs via the CLI something we want?
  2. If so, should we enhance `@Option` to support this or solve some other way?
  3. If we decide to enhance `@Option` or not, is this something we should consider making public?

Yes to all of these, I think. But not necessarily all at the same time. I'd implement #1 by doing #2, but perhaps not #3 right now (it would be nice if it were public however).

 

For more options, visit https://groups.google.com/d/optout.



--
Adam Murdoch
Gradle Co-founder
http://www.gradle.org
CTO Gradle Inc. - Gradle Training, Support, Consulting
http://www.gradle.com

Adam Murdoch

unread,
Feb 10, 2016, 7:16:06 PM2/10/16
to gradl...@googlegroups.com
On Thu, Feb 11, 2016 at 8:53 AM, Mark Vieira <ma...@gradle.com> wrote:
If I understand correctly '@Option'  already supports multiple arguments. But, due to some reason, there some asserts were added which were not allowing to use this feature. 

I believe you are conflating command line options (ex: gradle --info) with task options (ex. gradle dependencies --configuration compile). These are two completely separate things with different implementations. The @Option annotation only applies to the latter. Currently @Option does not support collection types.

My point being that we can potentially support the mutliple test filter use case without making changes to @Option. One argument to extending @Option to support collection types is that it might be useful for other things. That said, it's not a public API so that shouldn't be a motivation in this case. It's also not a trivial thing to implement. Handling multiple values means using a delimiter of some kind. We can't use a space because of ambiguity reasons. In the case of `gradle test --tests foo bar` is 'bar' a test filter or a task? Picking an arbitrary delimiter is problematic too because it might be a valid character for the parameter depending on the task implementation. 

Our CLI parsing already knows the difference between single and multi-valued options. You just specify the option multiple times, once for each value. I wouldn't change this, I would just reuse this.

 

For more options, visit https://groups.google.com/d/optout.



--

Mark Vieira

unread,
Feb 10, 2016, 7:33:22 PM2/10/16
to gradl...@googlegroups.com
Thanks for the feedback, Adam.

Denis, I believe your initial spike aligns well with Adam's comments. I'd say let's go ahead and open up a pull request. As for the test failures, the first one looks to be bogus and unrelated. The other two look to be legitimate. This stuff will all shake out in CI when the PR is created.

To view this discussion on the web visit https://groups.google.com/d/msgid/gradle-dev/CAMyicfWV%2B-257BxHkBadnX4PGs6qXF%3Dks%3Dmy8tWtzARWYw2W7g%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.
Reply all
Reply to author
Forward
0 new messages