add filtering with device pattern

[KazuCocoa]

Hi, there

I'd like to add --devicePattern as one of the arguments for the composer command.

a use case

• Connect some devices, include emulators and read devices, to one machine and we'd like to run instrumentation tests with only emulators.
  • The composer already has --devices but we should set all serial patterns. I'd like to set the pattern with Regex to make script simple.

java -jar composer-latest-version.jar \
--apk app/build/outputs/apk/example-debug.apk \
--test-apk app/build/outputs/apk/example-debug-androidTest.apk \
--test-package com.example.test \
--test-runner com.example.test.ExampleTestRunner \
--output-directory artifacts/composer-output \
--instrumentation-arguments key1 value1 key2 value2 \
--devicePattern "emulator.+" <= new
--verbose-output false

What do you think?

[artem-zinnatullin]

In general I would prefer to use only --devices, because with --device-pattern we have 2 flags that not only can be used one instead of another but also can be used together and result in non-obvious behavior…

I understand that filtering adb devices in bash with grep/awk and passing result to --devices is not very pleasant task, but it's possible.

Also since you have JVM on board you can use Kotlin Script to filter and prepare list of devices for Composer.

@yunikkk @dmitry-novikov @ming13 what are your thoughts?

[dmitry-novikov]

@artem-zinnatullin, I see no issues with wider range of possibilities to specify test devices. To resolve issue with "non-obvious behavior" I would propose to prohibit usage of both parameters together.

[yunikkk]

--devices option looks stronger for me, so I would suggest --devices to overwrite --device-pattern if both are specified.

[artem-zinnatullin]

But if you use --devices it means that you know device ids upfront, while --device-pattern is designed for cases when you don't know device ids…

[yunikkk]

So if you specify --devices - you know which of them you need, not much sense to add additional --device-pattern...

[KazuCocoa]

I appreciate your reviews! Should I ignore --device-pattern if both --devices and --device-patten are provided?

[yunikkk]

It would be nice, as now it's unclear how two options will work together...

[KazuCocoa]

enable --device-pattern if --devices is not passed

[artem-zinnatullin]

Mm, wait @yunikkk I would prefer to exit with error if both parameters are present, otherwise user might not realize that one of them is silently ignored!

[trevjonez]

the fail with error when both are present would be more appropriate from the plugin perspective as well. I will have to add both to the plugin and tend to prefer strict prevention of illegal configs over printing warnings and hoping someone sees it.

[artem-zinnatullin]

Yep, great point about plugin, @trevjonez!

[yunikkk]

@artem-zinnatullin - ok, that sounds even better than silently ignoring. @KazuCocoa thanks! Sorry for conflicting review comments=\

[KazuCocoa]

Thanks for kinda reviews!

[KazuCocoa]

thanks for merging and the useful tool!

[artem-zinnatullin]

Open Source for the win!

On Mon, Aug 14, 2017, 6:50 AM Kazuaki Matsuo notifications@github.com wrote:

thanks for merging and the useful tool!

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/gojuno/composer/pull/94#issuecomment-322195161, or mute the thread https://github.com/notifications/unsubscribe-auth/AA7B3LJkqp1KaIkeBHFnlqT47CblVn16ks5sYFCagaJpZM4Or3Xk .

[artem-zinnatullin]

Thank you for your contributions :)

On Mon, Aug 14, 2017, 8:51 AM Artem Zinnatullin artem.zinnatullin@gmail.com wrote:

Open Source for the win!

On Mon, Aug 14, 2017, 6:50 AM Kazuaki Matsuo notifications@github.com wrote:

thanks for merging and the useful tool!

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/gojuno/composer/pull/94#issuecomment-322195161, or mute the thread https://github.com/notifications/unsubscribe-auth/AA7B3LJkqp1KaIkeBHFnlqT47CblVn16ks5sYFCagaJpZM4Or3Xk .

[KazuCocoa]

@yunikkk Could you release the new version?

[yunikkk]

@KazuCocoa sure