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

Additional changes of the #473 #786

Merged
merged 5 commits into from
Dec 18, 2017
Merged

Additional changes of the #473 #786

merged 5 commits into from
Dec 18, 2017

Conversation

TikhomirovSergey
Copy link
Contributor

Change list

Additional changes of the #473

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

  • AuthenticatesByFinger was added to io.appium.java_client.android
  • Some improvements of the FingerPrintTest

alizelzele and others added 3 commits December 2, 2017 17:13
- AuthenticatesByFinger was added to `io.appium.java_client.android`
- Some improvements of the FingerPrintTest
@TikhomirovSergey
Copy link
Contributor Author

@alizelzele You can take a look at this PR and review it too.

service = AppiumDriverLocalService.buildDefaultService();
service.start();

if (service == null || !service.isRunning()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

service cannot be null here

Copy link
Contributor

Choose a reason for hiding this comment

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

@mykola-mokhnach it was copied from BaseAndroidTest and for sure it can not be null here :)

enterPasswordAndContinue(driver);
enterPasswordAndContinue(driver);
clickNext(driver);
driver.quit();
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rather move driver create/quit into before/after each method

Copy link
Member

@SrinivasanTarget SrinivasanTarget left a comment

Choose a reason for hiding this comment

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

@TikhomirovSergey I always get errors irrespective of API levels (23,25, 26) & UIA1/UIA2.
https://gist.github.com/SrinivasanTarget/416e82b0f13db0b237cc92a23a85cbf6

@mykola-mokhnach
Copy link
Contributor

@SrinivasanTarget Did you try to grant app permissions in capabilities?

@SrinivasanTarget
Copy link
Member

@mykola-mokhnach I tried with AutoGrantPermissions Caps and output is same on both UIA1/UIA2. Is it the same caps you are talking about?

@TikhomirovSergey
Copy link
Contributor Author

@SrinivasanTarget Everything is ok for me.
https://gist.github.com/TikhomirovSergey/5d389ed79c8433c524825f2aa5ffe2b1

Could you try such emulator

image

@mykola-mokhnach I am going to make some changes soon.

@SrinivasanTarget
Copy link
Member

@TikhomirovSergey It works with above config. But not in API 25 & 26.
screen shot 2017-12-13 at 10 35 10 pm

@alizelzele
Copy link
Contributor

@SrinivasanTarget since we are not using any test app and android changed its settings implementation on different APIs. I didn't find any other way to implement this test.

@TikhomirovSergey
Copy link
Contributor Author

@alizelzele @mykola-mokhnach @SrinivasanTarget
It seems I found a way to make the test stable. Will update this PR on this weekend

@TikhomirovSergey
Copy link
Contributor Author

TikhomirovSergey commented Dec 16, 2017

@SrinivasanTarget Could you try to run the test with the latest change. It seems I made it working.
@mykola-mokhnach The test was re-designed.

driver.fingerPrint(2);
try {
clickNext(driver);
driver.findElementById("com.android.settings:id/next_button").click();
Copy link
Member

Choose a reason for hiding this comment

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

Should be clickNext()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. Will improve today.

text.equals(androidElement.getText())).findFirst()
.orElseThrow(() ->
new NoSuchElementException(String.format("There is no element with the text '%s'", text)));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I was so used to localization testing that my mind automatically blocked using text to find an element :)
using text is much better and easier in this case 👍

@TikhomirovSergey
Copy link
Contributor Author

@SrinivasanTarget The improvement has been done

Copy link
Member

@SrinivasanTarget SrinivasanTarget left a comment

Choose a reason for hiding this comment

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

Looks good now

@TikhomirovSergey
Copy link
Contributor Author

@SrinivasanTarget Is the test stable on your environment?

@SrinivasanTarget
Copy link
Member

Yes @TikhomirovSergey

@TikhomirovSergey TikhomirovSergey merged commit e37f4e4 into appium:master Dec 18, 2017
@alizelzele
Copy link
Contributor

yeaaayyy, finally 🎉

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

Successfully merging this pull request may close these issues.

4 participants