Implemented driver status check. by anilkrverma · Pull Request #1153 · appium/java-client

anilkrverma · 2019-06-13T11:01:35Z

Change list

Feature: Add support for "driver.status()" - #1137

Types of changes

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

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

Details

Implemented driver status check as requested in Feature: Add support for "driver.status()" #1137 which returns AppiumServerStatus object containing a field for returning the version of running appium server.
Added a sample test to validate if the version is of form "x.y"

anilkrverma · 2019-06-13T11:30:54Z

The tests are failing even when no changes are made in the code.
Refer #1155 which has a change in Readme text only.

This failure shouldn't be related to the code implemented here.

jlipps

👍 @SrinivasanTarget what do you think?

src/main/java/io/appium/java_client/AppiumDriver.java

anilkrverma

@jlipps @mykola-mokhnach @saikrishna321 @SrinivasanTarget Please review and merge if it looks good.
We need to fix breaking CI tests separately.

jlipps

this looks ok to me though it'd be nice to get the changes without a bunch of additional formatting changes as well (unless @SrinivasanTarget thinks they are a good idea too)

SrinivasanTarget

Code looks good now. Can you keep styling as per the style guides we use?

anilkrverma · 2019-06-16T10:56:52Z

Code looks good now. Can you keep styling as per the style guides we use?

@SrinivasanTarget Current changes have been done through standard auto-formatting in Java. Any reference to style guides doc for my understanding so that I can update it accordingly and be proactive in my future commits?

SrinivasanTarget · 2019-06-16T11:02:03Z

@anilkrverma https://github.com/appium/java-client/blob/master/google-style.xml
Gradle task: ./gradlew clean checkstyleMain

anilkrverma · 2019-06-16T11:20:02Z

@anilkrverma https://github.com/appium/java-client/blob/master/google-style.xml
Gradle task: ./gradlew clean checkstyleMain

@SrinivasanTarget Pushed the changes after fixing the violations reported. Please review.

SrinivasanTarget · 2019-06-16T11:32:43Z

@anilkrverma Can you rebase your branch with the latest master once as a final nit. CI should be green after that.

anilkrverma · 2019-06-16T13:21:51Z

Hurray, CI is green now. :)

src/main/java/io/appium/java_client/AppiumDriver.java

anilkrverma · 2019-06-20T09:21:14Z

@SrinivasanTarget , @mykola-mokhnach Rebased from master. Please review.

anilkrverma · 2019-06-20T09:57:23Z

@SrinivasanTarget @mykola-mokhnach @jlipps @saikrishna321 We are getting below error again:
Error: /Users/vsts/agent/2.153.2/work/1/s/gradlew failed with return code: 1

It was working before, Did we broke Azure CI again or am I missing something here?

mykola-mokhnach · 2019-06-20T10:13:45Z

This does not look like a rebase

- Solves Issue: appium#1137

…instead of AppiumServerStatus class object. - Deleted AppiumServerStatus class as it is no longer needed.

- Empty line should be followed by <p> tag on the next line. 234 - First sentence of Javadoc is missing an ending period. 234 Ran ./gradlew clean checkstyleMain, BUILD is SUCCESSFUL now.

anilkrverma · 2019-06-20T15:53:49Z

This does not look like a rebase
Should be good now. @mykola-mokhnach

anilkrverma

@mykola-mokhnach @saikrishna321 @SrinivasanTarget @jlipps Hope this looks perfect now for getting merged with master. Please let me know if further changes are required.

src/main/java/io/appium/java_client/AppiumDriver.java

saikrishna321 · 2019-06-21T13:01:41Z

Thanks for addressing all the comments. Looks good👍

jlipps · 2019-06-21T16:27:28Z

🎉 congrats @anilkrverma!

anilkrverma · 2019-06-22T04:54:40Z

Thanks @jlipps , @saikrishna321 @SrinivasanTarget @mykola-mokhnach for all the support.

jlipps reviewed Jun 13, 2019

View reviewed changes

jlipps requested a review from SrinivasanTarget June 13, 2019 11:45

mykola-mokhnach reviewed Jun 13, 2019

View reviewed changes

src/main/java/io/appium/java_client/AppiumDriver.java Outdated Show resolved Hide resolved

anilkrverma commented Jun 16, 2019

View reviewed changes

jlipps approved these changes Jun 16, 2019

View reviewed changes

SrinivasanTarget requested changes Jun 16, 2019

View reviewed changes

SrinivasanTarget approved these changes Jun 16, 2019

View reviewed changes

anilkrverma force-pushed the master branch from 0b664a5 to b23ab2d Compare June 16, 2019 12:51

saikrishna321 approved these changes Jun 16, 2019

View reviewed changes

mykola-mokhnach reviewed Jun 16, 2019

View reviewed changes

src/main/java/io/appium/java_client/AppiumDriver.java Outdated Show resolved Hide resolved

anilkrverma added 5 commits June 20, 2019 20:03

- Implemented driver status check.

b42cee8

- Solves Issue: appium#1137

- Made version field private.

1b6297c

- Changed the getStatus() method implementation to return versionMap …

71eebcb

…instead of AppiumServerStatus class object. - Deleted AppiumServerStatus class as it is no longer needed.

Fixed below checkstyle violations:

0583b51

- Empty line should be followed by <p> tag on the next line. 234 - First sentence of Javadoc is missing an ending period. 234 Ran ./gradlew clean checkstyleMain, BUILD is SUCCESSFUL now.

- Updated getStatus() method to return statusMap.

1077885

anilkrverma force-pushed the master branch from 1f3a139 to 1077885 Compare June 20, 2019 14:38

anilkrverma commented Jun 21, 2019

View reviewed changes

mykola-mokhnach reviewed Jun 21, 2019

View reviewed changes

src/main/java/io/appium/java_client/AppiumDriver.java Outdated Show resolved Hide resolved

- Made return statement inline.

0fdcd50

mykola-mokhnach approved these changes Jun 21, 2019

View reviewed changes

saikrishna321 merged commit 3249bd6 into appium:master Jun 21, 2019

takeyaqa mentioned this pull request Jul 8, 2019

docs: Add java command examples appium/appium#12870

Merged

9 tasks

Uh oh!

Conversation

anilkrverma commented Jun 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Change list

Types of changes

Details

Uh oh!

anilkrverma commented Jun 13, 2019

Uh oh!

jlipps left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

anilkrverma left a comment

Choose a reason for hiding this comment

Uh oh!

jlipps left a comment

Choose a reason for hiding this comment

Uh oh!

SrinivasanTarget left a comment

Choose a reason for hiding this comment

Uh oh!

anilkrverma commented Jun 16, 2019

Uh oh!

SrinivasanTarget commented Jun 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

anilkrverma commented Jun 16, 2019

Uh oh!

SrinivasanTarget commented Jun 16, 2019

Uh oh!

anilkrverma commented Jun 16, 2019

Uh oh!

Uh oh!

anilkrverma commented Jun 20, 2019

Uh oh!

anilkrverma commented Jun 20, 2019

Uh oh!

mykola-mokhnach commented Jun 20, 2019

Uh oh!

anilkrverma commented Jun 20, 2019

Uh oh!

anilkrverma left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

saikrishna321 commented Jun 21, 2019

Uh oh!

jlipps commented Jun 21, 2019

Uh oh!

anilkrverma commented Jun 22, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

anilkrverma commented Jun 13, 2019 •

edited

Loading

SrinivasanTarget commented Jun 16, 2019 •

edited

Loading