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

Add device_locked? method #330

Merged
merged 1 commit into from May 4, 2015

Conversation

JaniJegoroff
Copy link
Member

This PR provides implementation for #257

Test is implemented for Android only since iOS support is not implemented in Appium yet:
https://github.com/appium/appium/blob/master/lib/devices/ios/ios-controller.js#L1217

Android test results

Finished in 3 mins 55 secs

107 runs, 148 assertions, 0 failures, 0 errors, 0 skips

Appium console output

Janis-MacBook-Pro:android_tests janijegoroff$ arc
[1] pry(main)> device_locked?
false
[2] pry(main)> lock 5
nil
[3] pry(main)> device_locked?
true
[4] pry(main)> press_keycode 82
true
[5] pry(main)> device_locked?
false
[6] pry(main)>

@bootstraponline
Copy link
Member

I think device_locked? is more descriptive. It also follows the pattern of the other end points a bit better. Other than that, the code looks good.

@bootstraponline bootstraponline added this to the v7 milestone Apr 29, 2015
@JaniJegoroff
Copy link
Member Author

Thanks, I will make review changes tomorrow.

@JaniJegoroff JaniJegoroff changed the title Add locked? method Add device_locked? method Apr 30, 2015
@bootstraponline
Copy link
Member

Thanks, please squash to one commit then it'll be ready to merge. Also on GitHub, there's no notification when a commit is pushed so after making a change it's a good idea to leave a comment so people know the PR has been updated.

@JaniJegoroff
Copy link
Member Author

Done, changes squashed to one commit.

bootstraponline added a commit that referenced this pull request May 4, 2015
@bootstraponline bootstraponline merged commit 9ea0754 into appium:master May 4, 2015
@bootstraponline
Copy link
Member

Thanks! merged.

@JaniJegoroff JaniJegoroff deleted the add-locked-method branch May 4, 2015 09:28
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

2 participants