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

#714 FIX #717

Merged
merged 5 commits into from
Sep 2, 2017
Merged

#714 FIX #717

merged 5 commits into from
Sep 2, 2017

Conversation

TikhomirovSergey
Copy link
Contributor

Change list

  • Update to Selenium 3.5.3
  • phantomJSdriver and htmlunit-drive were excluded
  • eclipse compiler was updated to 4.6.1
  • also AppiumElementLocator was improved

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

Dependencies of the html unit driver
https://mvnrepository.com/artifact/org.seleniumhq.selenium/htmlunit-driver/2.27

Dependencies of the phantom js driver
https://mvnrepository.com/artifact/com.codeborne/phantomjsdriver/1.4.3

Also something weird was foaund in AppiumElementLocator. I wonder why we couldn't find it before.

- Update to Selenium 3.5.3
- phantomJSdriver and htmlunit-drive were excluded
- eclipse compiler was updated to 4.6.1
@@ -21,8 +21,6 @@
import static io.appium.java_client.pagefactory.ThrowableUtil.isStaleElementReferenceException;


import com.google.common.base.Function;
Copy link
Contributor

Choose a reason for hiding this comment

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

I also see this import in several other sources - are they all expected?

Copy link
Contributor Author

@TikhomirovSergey TikhomirovSergey Sep 2, 2017

Choose a reason for hiding this comment

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

@mykola-mokhnach No
Except the AppiumFunction class. I improved sourses at my last commit

@@ -54,14 +54,18 @@ compileJava {
]
}

ext.seleniumVersion = '[3.5.2,3.5.2]'
ext.seleniumVersion = '[3.5.3,3.5.3]'
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the point to set a range, which includes only one item? Isn't it simply equal to '3.5.3'?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mykola-mokhnach The purpose is to make a valid pom.xml file. The version like 3.5+ is malformed for maven.

@mykola-mokhnach
Copy link
Contributor

I was also thinking about enforcing the version of guava library, since our module cannot work if it is older than 20

@SrinivasanTarget
Copy link
Member

@TikhomirovSergey As @mykola-mokhnach suggested i think we should force com.google.guava:guava:23.0 dependency.

- test improvement
@TikhomirovSergey
Copy link
Contributor Author

@SrinivasanTarget I will try it with the com.google.guava:guava:23.0 dependency.

}

return result;
if (result.size() == 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

return result.isEmpty() ? null : result;

@TikhomirovSergey
Copy link
Contributor Author

@SrinivasanTarget I think it is not necessary to force the com.google.guava:guava:23.0 because Selenium already depends on this version.

https://mvnrepository.com/artifact/org.seleniumhq.selenium/selenium-remote-driver/3.5.3

@TikhomirovSergey TikhomirovSergey merged commit 4e47690 into appium:master Sep 2, 2017
@SrinivasanTarget
Copy link
Member

I think it is not necessary to force the com.google.guava:guava:23.0 because Selenium already depends on this version.

Ok

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.

None yet

3 participants