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

Selenium 3.8.1 #638

Merged
merged 7 commits into from Dec 9, 2017

Conversation

Projects
None yet
4 participants
@rosolko
Collaborator

rosolko commented Nov 30, 2017

Proposed changes

  • Update dependencies:
    • webdrivermanager 1.7.2 -> 2.0.0
    • selenium-java 3.7.1 -> 3.8.1
    • selenium-server 3.7.1 -> 3.8.1
    • guava 23.0 -> 23.5
    • htmlunit-driver 2.28.1 -> 2.82.2
    • jetty-repacked 9.4.5 -> 9.4.7
    • jetty-server\servlet 9.4.5 -> 9.4.8
  • Update deprecated method using.
  • Removed Alert.authenticate using related to selenium changes.

Checklist

  • Checkstyle and unit tests pass locally with my changes by running gradle check chrome htmlunit command
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

rosolko added some commits Nov 30, 2017

Update dependencies:
* webdrivermanager 1.7.2 -> 2.0.0
* selenium-java 3.7.1 -> 3.8.0
* selenium-server 3.7.1 -> 3.8.0
* guava 23.0 -> 23.5
* htmlunit-driver 2.28.1 -> 2.82.2
* jetty-repacked 9.4.5 -> 9.4.7
* jetty-server\servlet 9.4.5 -> 9.4.8
Update deprecated method using.
Removed `Alert.authenticate` using related to selenium changes.
@asolntsev

This comment has been minimized.

Show comment
Hide comment
@asolntsev

asolntsev Dec 1, 2017

Contributor

@rosolko thank you for the PR! But please fix the FindBugs issue:

Dead store to domain in com.codeborne.selenide.impl.Navigator.navigateToAbsoluteUrl(String, String, String, String)

com.codeborne.selenide.impl.NavigatorIn method 
com.codeborne.selenide.impl.Navigator.navigateToAbsoluteUrl(String, String, String, String)
Local variable named domainAt Navigator.java:[line 51]
Contributor

asolntsev commented Dec 1, 2017

@rosolko thank you for the PR! But please fix the FindBugs issue:

Dead store to domain in com.codeborne.selenide.impl.Navigator.navigateToAbsoluteUrl(String, String, String, String)

com.codeborne.selenide.impl.NavigatorIn method 
com.codeborne.selenide.impl.Navigator.navigateToAbsoluteUrl(String, String, String, String)
Local variable named domainAt Navigator.java:[line 51]
@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Dec 1, 2017

Coverage Status

Coverage increased (+0.03%) to 64.314% when pulling bc755fe on rosolko:master into 3689ad6 on codeborne:master.

coveralls commented Dec 1, 2017

Coverage Status

Coverage increased (+0.03%) to 64.314% when pulling bc755fe on rosolko:master into 3689ad6 on codeborne:master.

@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Dec 1, 2017

Codecov Report

Merging #638 into master will increase coverage by 0.06%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #638      +/-   ##
============================================
+ Coverage     60.42%   60.49%   +0.06%     
  Complexity      771      771              
============================================
  Files           148      148              
  Lines          2752     2749       -3     
  Branches        272      270       -2     
============================================
  Hits           1663     1663              
+ Misses          983      981       -2     
+ Partials        106      105       -1
Impacted Files Coverage Δ Complexity Δ
...in/java/com/codeborne/selenide/impl/Navigator.java 68.85% <ø> (+3.22%) 21 <0> (ø) ⬇️
...rne/selenide/impl/DownloadFileWithHttpRequest.java 79.45% <100%> (ø) 15 <0> (ø) ⬇️
...om/codeborne/selenide/impl/WebElementSelector.java 65.51% <100%> (ø) 11 <0> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3689ad6...9e6a647. Read the comment docs.

codecov-io commented Dec 1, 2017

Codecov Report

Merging #638 into master will increase coverage by 0.06%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #638      +/-   ##
============================================
+ Coverage     60.42%   60.49%   +0.06%     
  Complexity      771      771              
============================================
  Files           148      148              
  Lines          2752     2749       -3     
  Branches        272      270       -2     
============================================
  Hits           1663     1663              
+ Misses          983      981       -2     
+ Partials        106      105       -1
Impacted Files Coverage Δ Complexity Δ
...in/java/com/codeborne/selenide/impl/Navigator.java 68.85% <ø> (+3.22%) 21 <0> (ø) ⬇️
...rne/selenide/impl/DownloadFileWithHttpRequest.java 79.45% <100%> (ø) 15 <0> (ø) ⬇️
...om/codeborne/selenide/impl/WebElementSelector.java 65.51% <100%> (ø) 11 <0> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3689ad6...9e6a647. Read the comment docs.

@rosolko

This comment has been minimized.

Show comment
Hide comment
@rosolko

rosolko Dec 1, 2017

Collaborator

@asolntsev Fixed.

Collaborator

rosolko commented Dec 1, 2017

@asolntsev Fixed.

@rosolko rosolko changed the title from Selenium 3.8.0 to Selenium 3.8.1 Dec 3, 2017

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Dec 3, 2017

Coverage Status

Coverage increased (+0.03%) to 64.314% when pulling 870e193 on rosolko:master into 3689ad6 on codeborne:master.

coveralls commented Dec 3, 2017

Coverage Status

Coverage increased (+0.03%) to 64.314% when pulling 870e193 on rosolko:master into 3689ad6 on codeborne:master.

@asolntsev

@rosolko Please couple of changes more.

@@ -68,9 +65,6 @@ protected void navigateToAbsoluteUrl(String url, String domain, String login, St
try {
WebDriver webdriver = getAndCheckWebDriver();
webdriver.navigate().to(url);
if (isIE() && !"".equals(login)) {
Selenide.switchTo().alert().authenticateUsing(new UserAndPassword(domain + login, password));
}

This comment has been minimized.

@asolntsev

asolntsev Dec 3, 2017

Contributor

@rosolko Why this code disappeared?

@asolntsev

asolntsev Dec 3, 2017

Contributor

@rosolko Why this code disappeared?

This comment has been minimized.

@rosolko

rosolko Dec 4, 2017

Collaborator

@asolntsev Because selenium remove this functionality in 3.8 version.
From selenium-java-3.8.0 changelogs:

Removed Alert.authenticate and supporting classes

Reference

@rosolko

rosolko Dec 4, 2017

Collaborator

@asolntsev Because selenium remove this functionality in 3.8 version.
From selenium-java-3.8.0 changelogs:

Removed Alert.authenticate and supporting classes

Reference

Show outdated Hide outdated src/main/java/com/codeborne/selenide/impl/WebElementSelector.java Outdated
@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Dec 4, 2017

Coverage Status

Coverage increased (+0.03%) to 64.314% when pulling 9e6a647 on rosolko:master into 3689ad6 on codeborne:master.

coveralls commented Dec 4, 2017

Coverage Status

Coverage increased (+0.03%) to 64.314% when pulling 9e6a647 on rosolko:master into 3689ad6 on codeborne:master.

@asolntsev asolntsev self-assigned this Dec 9, 2017

@asolntsev asolntsev added this to the 4.9 milestone Dec 9, 2017

@asolntsev asolntsev merged commit dfea2de into selenide:master Dec 9, 2017

4 checks passed

codecov/patch 100% of diff hit (target 60.42%)
Details
codecov/project 60.49% (+0.06%) compared to 3689ad6
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.03%) to 64.314%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment