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

Feat: Add support for viewmatcher #1293

Merged
merged 2 commits into from
Jan 17, 2020

Conversation

ccharnkij
Copy link
Contributor

Change list

Per appium/appium#13747, add support for viewmatcher along with test for datamatcher

Types of changes

  • No changes in production code.
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Details

Similar to dataMatcher, provides a functionality for viewMatcher. Also I noticed that dataMatcher had no test, so I added one for dataMatcher as well.

@jsf-clabot
Copy link

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Chanatan Charnkijtawarush seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.

Add support for viewmatcher along with test for datamatcher
@@ -106,6 +106,17 @@ public static By androidDataMatcher(final String dataMatcherString) {
return new ByAndroidDataMatcher(dataMatcherString);
}

/**
* This locator strategy is only available in Espresso Driver mode.
* @param viewMatcherString is a valid class chain locator string.
Copy link
Contributor

Choose a reason for hiding this comment

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

class chain?

Copy link
Contributor Author

@ccharnkij ccharnkij Jan 16, 2020

Choose a reason for hiding this comment

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

The sin of copy & paste...I got it from the dataMatcher. I'll add a new commit in real quick, for the dataMatcher too. My apologies...

@mykola-mokhnach
Copy link
Contributor

Please sign the CLA, so we could merge the PR. Thanks

Fix method doc for dataMatcher and viewMatcher
@ccharnkij
Copy link
Contributor Author

I believe the cla check is good.

@mykola-mokhnach
Copy link
Contributor

I believe the cla check is good.

the bot verifies the email address set to your local git config and compares it with the one used for CLA signing. Just make sure you created the PR using the correct local email

@mykola-mokhnach
Copy link
Contributor

The PR is fine, but I unfortunately cannot merge it unless the bot confirms the CLA is signed. Make sure the email address in your local github config is correct and recreate the PR

@ccharnkij
Copy link
Contributor Author

@mykola-mokhnach Doesn't this mean that the bot successfully confirms CLA is signed?

Screen Shot 2020-01-16 at 12 35 48 PM

@mykola-mokhnach
Copy link
Contributor

yes, this looks good. Although it is still weird why the bot didn't change the first comment. @jlipps do you know what the expected behaviour is?

@jlipps
Copy link
Member

jlipps commented Jan 17, 2020

Yeah expected behavior is only to approve the fixed commit. As long as the PR is green we should be good to go

@mykola-mokhnach
Copy link
Contributor

ok, thanks for checking this @jlipps

@mykola-mokhnach mykola-mokhnach merged commit 3573e23 into appium:master Jan 17, 2020
@ccharnkij ccharnkij deleted the add-viewmatcher branch January 17, 2020 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants