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.

Sign up for GitHub

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

Jump to bottom

Force resolution of selenium-webdriver to 4.10.0 #1541

Merged
merged 1 commit into from
Aug 3, 2023
Merged

Force resolution of selenium-webdriver to 4.10.0 #1541

merged 1 commit into from
Aug 3, 2023

Conversation

cwperks
Copy link
Member

@cwperks cwperks commented Aug 3, 2023

Description

Fixes an issue with the Integration Tests that were failing with selenium-webdriver 4.11.1. The tests had a bug on determining browser path that would result in it eventually failing with a message InvalidArgumentError: binary is not a Firefox executable. See details on linked issue.

Category

[Enhancement, New feature, Bug fix, Test fix, Refactoring, Maintenance, Documentation]

Test fix

Issues Resolved

#1540

Check List

  • New functionality includes testing
  • New functionality has been documented
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@codecov
Copy link

codecov bot commented Aug 3, 2023

Codecov Report

Merging #1541 (bdb2bde) into main (91faa3d) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1541   +/-   ##
=======================================
  Coverage   66.06%   66.06%           
=======================================
  Files          93       93           
  Lines        2328     2328           
  Branches      310      310           
=======================================
  Hits         1538     1538           
  Misses        722      722           
  Partials       68       68

@RyanL1997 RyanL1997 added the backport 2.x backport to 2.x branch label Aug 3, 2023
RyanL1997
RyanL1997 reviewed Aug 3, 2023
View reviewed changes
Copy link
Collaborator

@RyanL1997 RyanL1997 left a comment

Choose a reason for hiding this comment

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

Thanks for the fix @cwperks ! I just have a quick question.

package.json
"selfsigned": "^2.0.1",
"typescript": "4.0.2"
},
"dependencies": {
"@hapi/cryptiles": "5.0.0",
"@hapi/wreck": "^17.1.0",
"html-entities": "1.3.1"
},
"resolutions": {
"selenium-webdriver": "4.10.0"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: Was there any pkgs dpending on the selenium-webdriver with different versions? Just trying to figure out why we are using the selective dependencies resolution here.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is forcing to 4.10.0 due to a suspected bug in selenium. See the versions here: https://www.npmjs.com/package/selenium-webdriver?activeTab=versions

4.11.0 was published 3 days ago and 4.11.1 2 days ago. We can try removing this forced resolution on future versions of selenium. I'm going to look through the selenium issue list to look for any known issues: https://github.com/SeleniumHQ/selenium/issues

Copy link
Member Author

Choose a reason for hiding this comment

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

I suspect that the error comes from this PR in Selenium, but I don't know enough about selenium to be able to determine why it produces the error we are seeing.

@stephen-crawford stephen-crawford merged commit 95f316a into opensearch-project:main Aug 3, 2023
opensearch-trigger-bot bot pushed a commit that referenced this pull request Aug 3, 2023
@cwperks @github-actions
Force resolution of selenium-webdriver to 4.10.0 (#1541)
9367824 
Signed-off-by: Craig Perkins <[email protected]>
(cherry picked from commit 95f316a)
@opensearch-trigger-bot opensearch-trigger-bot bot mentioned this pull request Aug 3, 2023
Merged
cwperks added a commit that referenced this pull request Aug 3, 2023
@opensearch-trigger-bot @cwperks
Force resolution of selenium-webdriver to 4.10.0 (#1541) (#1542)
d8f5851 
Signed-off-by: Craig Perkins <[email protected]>
(cherry picked from commit 95f316a)

Co-authored-by: Craig Perkins <[email protected]>
samuelcostae pushed a commit to samuelcostae/security-dashboards-plugin that referenced this pull request Aug 10, 2023
@cwperks @samuelcostae
Force resolution of selenium-webdriver to 4.10.0 (opensearch-project#…
0a65206 
…1541)

Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Sam <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RyanL1997 RyanL1997 RyanL1997 approved these changes

@stephen-crawford stephen-crawford stephen-crawford approved these changes

@cliu123 cliu123 Awaiting requested review from cliu123

@DarshitChanpura DarshitChanpura Awaiting requested review from DarshitChanpura DarshitChanpura is a code owner

@davidlago davidlago Awaiting requested review from davidlago

@peternied peternied Awaiting requested review from peternied

Assignees
No one assigned
Labels
backport 2.x backport to 2.x branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cwperks @stephen-crawford @RyanL1997