add finger print command to android Driver #473

alizelzele · 2016-09-18T06:40:34Z

Change list

added finger print function to android driver. defined the command url and helper method, and created an Action shortcut to use it.

Types of changes

What types of changes are you proposing/introducing to Java client?
Put an x in the boxes that apply

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

it is a simple function to activate emulators fingerprint sensor. the command is like driver.fingerPrint(x) where x is the finger number (1 to 10). this command is created based on add finger print to route

saikrishna321 · 2016-09-19T12:53:48Z

@alizelzele can you please add tests?

TikhomirovSergey · 2016-09-19T13:59:11Z

@saikrishna321 +1
@alizelzele Could you add some tests?

saikrishna321 · 2016-09-19T14:35:31Z

@TikhomirovSergey @alizelzele what is the use-case for this PR?

TikhomirovSergey · 2016-09-19T16:23:10Z

@saikrishna321 This is why I'm asking for tests.

alizelzele · 2016-09-20T07:09:20Z

@saikrishna321 @TikhomirovSergey since android 6.0 there is a finger print sensor installed on some devices for security + some applications already started supporting fingerprint instead of their login method.

I am a bit busy now, will try to add test for it ASAP.
Test should activate fingerprint security in the setting, and disable security after that, since if it stays there, all other tests will fail (with default appium setting, which does not wipe the data on session start)

SrinivasanTarget

By Saying fingerprintID,
Are you sure it is finger number from 1 to 10? If so how can a device or a emulator knows finger 1 to 10 prints of owner?
AFIK it should be the saved fingerprint's (counts) on your device/emulator to access it in security settings.Also i don't think any device that supports 10 prints to be stored. Please update enough information in this PR or could be links to clarify these q's.

alizelzele · 2016-09-21T02:38:29Z

@SrinivasanTarget this function is only for emulators. Android 6 support fingerprint , and emulator supports that behavior by allowing you to use 10 predefined fingerprints. You just select which finger id you want to touch the sensor and it will touch it for you :-).
Sorry if I was not clear about functionality. I added an screenshot from emulator options to make it a bit more clear.

SrinivasanTarget · 2016-09-21T14:24:41Z

@alizelzele Ok Thanks for your clarification. Few remarks,

1)Adding tests as mentioned above.
2)https://github.com/appium/java-client/pull/473/files#diff-9fde7d141e27783a303930113c99c211R62 should be fingerPrint i.e P should be case-sensitive.
3)I think gradle check will be failing. Refer this: https://github.com/appium/java-client/blob/master/docs/Note-for-developers.md. Fix the checkstyle errors.
4) javadoc is not clear. Mention it like this,

Authenticate users by using their fingerprint scans on supported emulators.
@param fingerprintId    fingerprints stored in Android Keystore system.

SrinivasanTarget · 2016-10-02T17:01:58Z

@alizelzele Did you got a chance to look into this?

alizelzele · 2016-10-08T06:31:04Z

@SrinivasanTarget

test method added
changed fingerprint to fingerPrint
update code with google code style and pass gradle check
update the documentation as you asked

p.s: sorry for the big delay. I were busy at work since it was close to national holiday in china, and after that it was (obviously) national holidays. (and more obvious, I am working in china 😄 )

SrinivasanTarget · 2016-10-08T06:47:52Z

@alizelzele Cool will review it soon.

SrinivasanTarget · 2016-10-18T16:29:47Z

Nice Catch @codacy-bot. @alizelzele Can you please fix this?

alizelzele · 2016-10-21T07:02:15Z

src/test/java/io/appium/java_client/android/FingerPrintTest.java

+    /**
+     * add a new finger print to security.
+     */
+    @Test public void pressKeyCodeTest() throws Exception {


@SrinivasanTarget the code is suppose to throw exception if the test fails. I add try catch to whole test body and assert fail on failing :-)

use the @Test(expected = MyException.class) syntax for such case

@mykola-mokhnach throwing exception is not the expected behavior. if it throws any that means it failed. try catch is there to prevent throwing exception on test fail (as asked by @SrinivasanTarget )

ok, perhaps it makes sense to add some nice description to the fail method in such case

alizelzele · 2016-10-21T07:05:10Z

src/test/java/io/appium/java_client/android/FingerPrintTest.java

+        service.start();
+
+        if (service == null || !service.isRunning()) {
+            throw new RuntimeException("An appium server node is not started!");


@SrinivasanTarget This part of code is duplicate of io.appium.java_client.android.BaseAndroidTest. I will use another RuntimeException to throw, Don't think adding a new Exception class is required

SrinivasanTarget · 2016-11-17T18:10:44Z

src/test/java/io/appium/java_client/android/FingerPrintTest.java

+        capabilities.setCapability("appPackage", "com.android.settings");
+        capabilities.setCapability("appActivity", activity);
+        return (AndroidDriver) new AndroidDriver<AndroidElement>(service.getUrl(), capabilities);
+        //        return (AndroidDriver) new AndroidDriver<AndroidElement>(new URL("http://localhost:4723/wd/hub"), capabilities);


Comment should be removed here.

SrinivasanTarget · 2016-11-28T12:17:14Z

@alizelzele Can you sign CLA?

SrinivasanTarget · 2016-11-28T12:31:08Z

src/main/java/io/appium/java_client/android/AndroidDeviceActionShortcuts.java

@@ -16,6 +16,7 @@

 package io.appium.java_client.android;



AndroidDeviceActionShortcuts API is deprecated.

alizelzele · 2016-12-01T05:11:25Z

@SrinivasanTarget I believe i already signed the cla ( maybe with another email).
can you point me to CLA link ( I could not find it in docs ) :">

jsf-clabot · 2016-12-01T05:21:28Z

All committers have signed the CLA.

SrinivasanTarget · 2016-12-01T05:22:08Z

@alizelzele Above is the link now. But still there are some work left for you to complete.

SrinivasanTarget · 2016-12-01T05:23:20Z

@alizelzele Please refer this: #509

alizelzele · 2016-12-01T05:24:38Z

@SrinivasanTarget signed it. It was new! I guess I mistake it with Selion CLI :-)
remove the comment and update the code for deprecated interface.

TikhomirovSergey

@alizelzele I think that the FingerPrintTest may be more simle. Also there are merge conflicts.

TikhomirovSergey · 2017-08-07T20:14:15Z

src/test/java/io/appium/java_client/android/FingerPrintTest.java

+
+import java.util.concurrent.TimeUnit;
+
+public class FingerPrintTest {


@alizelzele Why don't you like to extend the BaseAndroidTest?

@TikhomirovSergey because test needs access to service created in BaseAndroidTest which is private.

TikhomirovSergey

The FingerPrintTest needs to be improved. Also it needs to resolve conflicts.

TikhomirovSergey · 2017-08-07T20:18:26Z

src/test/java/io/appium/java_client/android/FingerPrintTest.java

+        DesiredCapabilities capabilities = new DesiredCapabilities();
+        capabilities.setCapability(MobileCapabilityType.DEVICE_NAME, "Android Emulator");
+        capabilities.setCapability("appPackage", "com.android.settings");
+        capabilities.setCapability("appActivity", activity);


@alizelzele Maybe ApiDemos has convenient functionality. Have you tried to play with it?

@TikhomirovSergey searched it. did not find anything fingerprint related to use.

saikrishna321 · 2017-08-08T00:00:59Z

@TikhomirovSergey this feature is part of Android base driver server from version 2.0.16

fuzzybinary · 2017-11-17T18:46:00Z

What's the current state of this feature / PR? Does it need to be redone or just have the comments addressed?

alizelzele · 2017-11-21T02:50:53Z

Sorry guys, I did not find any time to reply and fix the issues.

the test needs to enable lock on device and make sure it is unlocked after the test, which added a bit of complexity to code. I was going to update appium-unlock to support sending passKey when unlocking to make things more clear and easier to handle.

I am rebasing and fixing merge issues now. but if I remember correctly the xpath in test were not valid anymore. will give it a shot

alizelzele · 2017-11-21T05:37:43Z

Stupid github (me), I push without committing my changes first, and github automatically closed the PR. 🤦‍♂️

alizelzele · 2017-11-21T05:43:56Z

update the code and fixed merge conflicts. test run correctly and successfully on this avd:

 Device: Nexus 5 (Google)
   Path: ??? :)
 Target: Default
         Based on: Android 6.0 (Marshmallow) Tag/ABI: default/x86_64
   Skin: 1080x1920

which emulator config you are using that the test in failing on? since test is written using xpath and it is also running system applications, it is a bit (very) fragile. did not find any other way to do it though.

SrinivasanTarget · 2017-11-21T06:01:53Z

@alizelzele Thanks for changes. we will review it soon today.

I have started to review it again. I am going to provide the feedback ASAP

TikhomirovSergey

Hi @alizelzele Thank you very much for update

I was trying to run the test on different emulators with different API levels. The result was the same NoSuchElementException. could you provide some information to make the test running green or please make some improvements

alizelzele · 2017-11-29T03:09:53Z

@TikhomirovSergey It works on my system 😀 😁 😁
I will try to improve it (although I am still not sure if anything else can be used instead of xpath.)
can you give me AVD details you used? maybe I can start with error you get and improve on that front.

alizelzele · 2017-12-02T09:53:49Z

@TikhomirovSergey likely on Windows emulator there is another additional step when you want to enable security!

I updated the test, add more comment and test it. one issue on my system is that for some reason following command and thus appium was not able to trigger fingerprint:

adb emu finger touch 1

but I test the code using debugger and it is working also on windows too. hope you won't have the issue with SDK.

@alizelzele

@alizelzele ok. I have started to check it again.

TikhomirovSergey

@alizelzele I can approve this PR. But I want to propese some additional changes at the additional PR. I am going to propose it ASAP

h2904 · 2017-12-12T07:32:31Z

Hey guys,
so I am trying to automate a flow which involves fingerprint authentication, Once I register fingerprint on android simulator ,navigate to the app's login page and start to enter credentials, webdriver throws a exception saying unhandled alert present, even though there is no alert shown, the text of the alert is (which i got from alert.getText) [{"touchIdRequested":false,"credentials":{}}]" , If I remove the fingerprint registration from simulator i do not get the error, and am able to successfully login. The login page is in webView Context

other environment details:
Platform: Mac OS X
Appium:1.7.1
Android Simulator version:8.1.0(Pixel)
Android driver version:3.3.0

Please let me know if you have any information regarding the issue.

Additional changes of the #473

TikhomirovSergey added the enhancement label Sep 19, 2016

SrinivasanTarget requested changes Sep 20, 2016

View reviewed changes

SrinivasanTarget added this to the 5.0.0 milestone Sep 25, 2016

alizelzele force-pushed the finger-print branch 3 times, most recently from c3131e6 to 0473a25 Compare October 8, 2016 06:20

alizelzele commented Oct 21, 2016

View reviewed changes

alizelzele force-pushed the finger-print branch from 0473a25 to ec18611 Compare October 21, 2016 07:07

SrinivasanTarget reviewed Nov 17, 2016

View reviewed changes

SrinivasanTarget added the Android label Nov 17, 2016

SrinivasanTarget reviewed Nov 28, 2016

View reviewed changes

alizelzele force-pushed the finger-print branch from ec18611 to 3f07e40 Compare December 1, 2016 05:21

TikhomirovSergey reviewed Aug 7, 2017

View reviewed changes

TikhomirovSergey previously requested changes Aug 7, 2017

View reviewed changes

TikhomirovSergey modified the milestones: 5.1.0, 5.0.0 Aug 28, 2017

alizelzele closed this Nov 21, 2017

alizelzele force-pushed the finger-print branch from 3f07e40 to 5dab23d Compare November 21, 2017 05:30

alizelzele reopened this Nov 21, 2017

alizelzele force-pushed the finger-print branch from 01fe970 to f981200 Compare November 21, 2017 09:26

TikhomirovSergey previously requested changes Nov 27, 2017

View reviewed changes

alizelzele force-pushed the finger-print branch from f981200 to 5ecaa95 Compare December 2, 2017 08:55

add finger print command to android Driver

c8946c1

alizelzele force-pushed the finger-print branch from 5ecaa95 to c8946c1 Compare December 2, 2017 09:15

appium deleted a comment Dec 2, 2017

TikhomirovSergey approved these changes Dec 10, 2017

View reviewed changes

TikhomirovSergey removed the on hold label Dec 11, 2017

TikhomirovSergey mentioned this pull request Dec 11, 2017

Additional changes of the #473 #786

Merged

4 tasks

TikhomirovSergey merged commit c8946c1 into appium:master Dec 18, 2017

TikhomirovSergey added a commit that referenced this pull request Dec 18, 2017

Merge pull request #786 from TikhomirovSergey/alizelzele-finger-print

e37f4e4

Additional changes of the #473

alizelzele deleted the finger-print branch December 19, 2017 05:45


		import java.util.concurrent.TimeUnit;

		public class FingerPrintTest {

add finger print command to android Driver #473

add finger print command to android Driver #473

Conversation

alizelzele commented Sep 18, 2016 • edited Loading

Change list

Types of changes

Details

saikrishna321 commented Sep 19, 2016

TikhomirovSergey commented Sep 19, 2016

saikrishna321 commented Sep 19, 2016

TikhomirovSergey commented Sep 19, 2016

alizelzele commented Sep 20, 2016

SrinivasanTarget left a comment

Choose a reason for hiding this comment

alizelzele commented Sep 21, 2016

SrinivasanTarget commented Sep 21, 2016

SrinivasanTarget commented Oct 2, 2016

alizelzele commented Oct 8, 2016

SrinivasanTarget commented Oct 8, 2016

SrinivasanTarget commented Oct 18, 2016

alizelzele Oct 21, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SrinivasanTarget commented Nov 28, 2016

Choose a reason for hiding this comment

alizelzele commented Dec 1, 2016

jsf-clabot commented Dec 1, 2016 • edited Loading

SrinivasanTarget commented Dec 1, 2016

SrinivasanTarget commented Dec 1, 2016

alizelzele commented Dec 1, 2016

TikhomirovSergey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TikhomirovSergey left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

saikrishna321 commented Aug 8, 2017

fuzzybinary commented Nov 17, 2017

alizelzele commented Nov 21, 2017

alizelzele commented Nov 21, 2017

alizelzele commented Nov 21, 2017

SrinivasanTarget commented Nov 21, 2017

TikhomirovSergey left a comment • edited Loading

Choose a reason for hiding this comment

alizelzele commented Nov 29, 2017

alizelzele commented Dec 2, 2017

TikhomirovSergey left a comment • edited Loading

Choose a reason for hiding this comment

h2904 commented Dec 12, 2017

alizelzele commented Sep 18, 2016 •

edited

Loading

alizelzele Oct 21, 2016 •

edited

Loading

jsf-clabot commented Dec 1, 2016 •

edited

Loading

TikhomirovSergey left a comment •

edited

Loading

TikhomirovSergey left a comment •

edited

Loading

TikhomirovSergey left a comment •

edited

Loading