Skip to content
This repository has been archived by the owner on Feb 11, 2022. It is now read-only.

Adds feature to filter monkey by intent category #90

Merged
merged 8 commits into from
Mar 8, 2016

Conversation

jszmltr
Copy link
Contributor

@jszmltr jszmltr commented Mar 4, 2016

This PR Adds feature to filter monkey by intent category.

It's done by adding '-c' INTENT_FILTER to the shell command.
Default to the monkeys normal categories if -c not specified

Paired with @blundell

 - moves monkey testing into new activity with specified intent ONLY_ME
@@ -15,12 +16,29 @@ class Monkey extends AdbTask {

@TaskAction
void exec() {
def arguments = ['shell', 'monkey', '-p', packageName, '-v', getEvents()]
def arguments = ['shell', 'monkey', '-p', packageName]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might just split the '-p' and packageName ones out onto the next line too, for consistency
e.g.

arguments += ['-p', packageName]

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 for separating the flags with their inputs

def arguments = [
    'shell', 
    'monkey', 
    '-p', packageName
]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@ouchadam
Copy link
Contributor

ouchadam commented Mar 7, 2016

few small comments, agree with @jonreeve otherwise LGTM 👍

}

@TaskAction
void exec() {
def arguments = ['shell', 'monkey', '-p', packageName, '-v', getEvents()]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@jonreeve
Copy link

jonreeve commented Mar 8, 2016

LGTM 👍

@ouchadam
Copy link
Contributor

ouchadam commented Mar 8, 2016

👍

ouchadam added a commit that referenced this pull request Mar 8, 2016
Adds feature to filter monkey by intent category
@ouchadam ouchadam merged commit 8dcd3c2 into develop Mar 8, 2016
@ouchadam ouchadam deleted the category_filter branch March 8, 2016 17:35
@@ -5,12 +5,16 @@ import org.gradle.api.Project

public class AndroidCommandPluginExtension {

static final String CATEGORIES_DEFAULT = ''
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What a default is an empty category? Would that work at all with the '-c' ?
Should be inlined imho.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah from the monkey docs an empty category is treated like the default categories. Right now the default is category of launcher or monkey.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't set the -c when there is no category.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but doing a blank -c allows the adb command to use the default categories (the same as not setting it)

deviceId ?: defaultDeviceId()
}

// prefer system property over direct setting to enable commandline arguments
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is still a valuable comment. Why did you remove it? Is it obvious that the ordering here is related to overriding properties on the command line?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments should be removed only if obsolete. This is not the case, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When changing from double elvis operator I find it quiet obvious what the if else statement does.
If we want to go back to one-line solution then yes, I think that comment is still needed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants