Skip to content

WAR-724 - To test if all the available selenium functionality in Warrior is compatible with Selenium 3.0#62

Merged
satyakrish merged 6 commits into
warriorframework:developfrom
nalagapp:feature/WAR-724-Selenium3support
Oct 4, 2017
Merged

WAR-724 - To test if all the available selenium functionality in Warrior is compatible with Selenium 3.0#62
satyakrish merged 6 commits into
warriorframework:developfrom
nalagapp:feature/WAR-724-Selenium3support

Conversation

@nalagapp

@nalagapp nalagapp commented Jul 4, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor
  1. Added the support for Firefox version above 47 using geckodriver and Selenium 3.x
  2. Added the support for Chrome browser using chromedriver

Results files are attached in JIRA ticket.

How-to-test:
Steps are added in user doc and attached in JIRA ticket

@nalagapp
nalagapp force-pushed the feature/WAR-724-Selenium3support branch from 8e74d11 to 2d27769 Compare July 6, 2017 06:12
@nalagapp nalagapp changed the title WAR-724 code changes WAR-724 - To test if all the available selenium functionality in Warrior is compatible with Selenium 3.0 Jul 6, 2017
@venkat1077
venkat1077 requested a review from navvrnd July 31, 2017 06:37
navvrnd
navvrnd reviewed Jul 31, 2017
View reviewed changes
Comment thread warrior/Framework/ClassUtils/WSelenium/browser_mgmt.py Outdated
def _make_chrome(self, webdriver_remote_url, desired_capabilities, profile_dir):
try:
if webdriver_remote_url:
browser = \

@navvrnd navvrnd Jul 31, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

no need of backslash as the next line can also be put here without affecting readability or pylint scores.

navvrnd
navvrnd previously approved these changes Jul 31, 2017
View reviewed changes

@navvrnd navvrnd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks fine.

@bjrr
bjrr requested review from brettleehari and removed request for sanikakulkarni and satyakrish September 13, 2017 20:35
@priyaarun23

priyaarun23 commented Sep 18, 2017

Copy link
Copy Markdown
Contributor

Tested the functionality from user perspective. In Windows its working fine, in Linux URL error has been noted.
@nalagapp has raised SUPWAR-173 for the same as this needs to be checked in Warrior Framework side.

priyaarun23
priyaarun23 previously approved these changes Sep 18, 2017
View reviewed changes
@brettleehari

brettleehari commented Sep 22, 2017

Copy link
Copy Markdown
Contributor

@bjrr
As communicated through e-mail, The pull request test hits the bug and fails with WAR-1141, (#66)
Request to review and merge WAR-1141 to complete the review test of this functionality and do a clean exit. We have tested with WAR-1141 fix and it works fine, so if WAR-1141 is merged then this is good to go!

brettleehari
brettleehari suggested changes Sep 22, 2017
View reviewed changes

@brettleehari brettleehari left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Dependent on WAR-1141

brettleehari
brettleehari previously approved these changes Oct 2, 2017
View reviewed changes
@satyakrish

satyakrish commented Oct 4, 2017

Copy link
Copy Markdown
Contributor

resolving merge conflicts undid the approvals given by priya and arvind. merging now using admin rights

@satyakrish
satyakrish merged commit 84265e0 into warriorframework:develop Oct 4, 2017
kaheichan added a commit that referenced this pull request Dec 20, 2017
@kaheichan
Revert "Merge pull request #62 from nalagapp/feature/WAR-724-Selenium…
344f84b 
…3support"

This reverts commit 84265e0, reversing
changes made to d16e336.
kaheichan
kaheichan reviewed Dec 20, 2017
View reviewed changes
Comment thread warrior/Actions/SeleniumActions/browser_actions.py
if type == "firefox":
ff_profile = self.browser_object.\
set_firefoxprofile(proxy_ip, proxy_port)
if binary != "" and gecko_path != "":

@kaheichan kaheichan Dec 20, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

gecko_path and binary value may be None

@brettleehari brettleehari Dec 21, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These issues were found and addressed in the PR : #317

@kaheichan kaheichan Dec 28, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The PR mentioned is looking for gecko_path and binary in system, but gecko_path and binary should be in browser, which is a child of system
so it isn't addressed correctly

kaheichan
kaheichan reviewed Dec 20, 2017
View reviewed changes
Comment thread warrior/Framework/ClassUtils/WSelenium/browser_mgmt.py
print_error("Please provide path for geckodriver executable")
elif "Expected browser binary location" in str(e):
print_error("Please provide path of firefox executable")

@kaheichan kaheichan Dec 20, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No Else block here to log other possible error

@brettleehari brettleehari Dec 21, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This issue of general exception is found and addressed in the PR : #317
The else part is a good find.

@kaheichan kaheichan Dec 27, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The issue here is about when WebDriverException occurs, the error message may not get printed because of the lack of else statement in this block
The PR mentioned wrapped everything in one general except block and shouldn't count as addressed since it simply discard the error specific messages here
Also it force spawn a firefox browser without printing out any error message...

kaheichan
kaheichan reviewed Dec 20, 2017
View reviewed changes
Comment thread warrior/Framework/ClassUtils/WSelenium/browser_mgmt.py
print_error("Please provide path of firefox executable")

def _make_chrome(self, webdriver_remote_url, desired_capabilities,
profile_dir, binary, gecko_path):

@kaheichan kaheichan Dec 20, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No explanation about gecko_path appearance in here
chrome doesn't need gecko_path and this is passed with no use

This was referenced Dec 20, 2017
Closed
Merged
@kaheichan

kaheichan commented Jan 4, 2018

Copy link
Copy Markdown
Contributor

This PR is being reverted and rework in #334

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bjrr bjrr Awaiting requested review from bjrr
4 more reviewers
@brettleehari brettleehari brettleehari left review comments
@kaheichan kaheichan kaheichan left review comments
@navvrnd navvrnd navvrnd left review comments
@priyaarun23 priyaarun23 priyaarun23 left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@nalagapp nalagapp

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@nalagapp @priyaarun23 @brettleehari @satyakrish @kaheichan @navvrnd @sanikakulkarni