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

Fix pattern unlock method #489

Merged
merged 1 commit into from
Mar 18, 2019
Merged

Fix pattern unlock method #489

merged 1 commit into from
Mar 18, 2019

Conversation

IgalKorb
Copy link
Contributor

added the use of the last position (and now the actions are relative to the previous rather than relative to {0,0}

added the use of the last position (and now the actions are relative to the previous rather than relative to {0,0}
@jsf-clabot
Copy link

jsf-clabot commented Jan 16, 2019

CLA assistant check
All committers have signed the CLA.

@mykola-mokhnach
Copy link
Contributor

@vrunoa Can you please check the PR?

@mykola-mokhnach
Copy link
Contributor

@IgalKorb Can you please update the unit test as well?

@IgalKorb
Copy link
Contributor Author

IgalKorb commented Jan 19, 2019 via email

@imurchie
Copy link
Contributor

No comment on the change itself, since I'm not sure about it. But the test can be fixed with the following:

diff --git a/test/unit/unlock-helper-specs.js b/test/unit/unlock-helper-specs.js
index 4deb609..1bbdb70 100644
--- a/test/unit/unlock-helper-specs.js
+++ b/test/unit/unlock-helper-specs.js
@@ -309,33 +309,34 @@ describe('Unlock Helpers', function () {
         }
       });
     });
     it('should verify pattern gestures moves to non consecutives pins', function () {
-      let keys = ['7', '2', '9', '8', '5', '6', '1', '4', '3'];
-      let actions = helpers.getPatternActions(keys, {x: 0, y: 0}, 1);
+      const keys = ['7', '2', '9', '8', '5', '6', '1', '4', '3'];
+      const actions = helpers.getPatternActions(keys, {x: 0, y: 0}, 1);
+
       // Move from pin 7 to pin 2
-      actions[1].options.x.should.equal(1);
-      actions[1].options.y.should.equal(-2);
+      actions[1].options.x.should.equal(2);
+      actions[1].options.y.should.equal(1);
       // Move from pin 2 to pin 9
-      actions[2].options.x.should.equal(1);
-      actions[2].options.y.should.equal(2);
+      actions[2].options.x.should.equal(3);
+      actions[2].options.y.should.equal(3);
       // Move from pin 9 to pin 8
-      actions[3].options.x.should.equal(-1);
-      actions[3].options.y.should.equal(0);
+      actions[3].options.x.should.equal(2);
+      actions[3].options.y.should.equal(3);
       // Move from pin 8 to pin 5
-      actions[4].options.x.should.equal(0);
-      actions[4].options.y.should.equal(-1);
+      actions[4].options.x.should.equal(2);
+      actions[4].options.y.should.equal(2);
       // Move from pin 5 to pin 6
-      actions[5].options.x.should.equal(1);
-      actions[5].options.y.should.equal(0);
+      actions[5].options.x.should.equal(3);
+      actions[5].options.y.should.equal(2);
       // Move from pin 6 to pin 1
-      actions[6].options.x.should.equal(-2);
-      actions[6].options.y.should.equal(-1);
+      actions[6].options.x.should.equal(1);
+      actions[6].options.y.should.equal(1);
       // Move from pin 1 to pin 4
-      actions[7].options.x.should.equal(0);
-      actions[7].options.y.should.equal(1);
+      actions[7].options.x.should.equal(1);
+      actions[7].options.y.should.equal(2);
       // Move from pin 4 to pin 3
-      actions[8].options.x.should.equal(2);
-      actions[8].options.y.should.equal(-1);
+      actions[8].options.x.should.equal(3);
+      actions[8].options.y.should.equal(1);
     });
   });
   describe('patternUnlock', withMocks({driver, helpers, adb, asyncbox}, (mocks) => {

This also should be rebased against master.

@vrunoa
Copy link
Contributor

vrunoa commented Jan 30, 2019

@IgalKorb I'll try to review it by the end of the weekend.

@dpgraham
Copy link
Contributor

@vrunoa did you have a chance to look at this?

@IgalKorb
Copy link
Contributor Author

@vrunoa @dpgraham Could you please review it? :D

@dpgraham
Copy link
Contributor

@imurchie @mykola-mokhnach do either of you have any objections to just merging this with the failing test and then I can apply @imurchie 's patch immediately after?

@imurchie
Copy link
Contributor

No objections from me.

@dpgraham dpgraham merged commit 36e6e52 into appium:master Mar 18, 2019
@dpgraham dpgraham mentioned this pull request Mar 18, 2019
@IgalKorb IgalKorb deleted the patch-1 branch April 10, 2019 10:27
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.

6 participants