Skip to content

Force resolution of selenium-webdriver to 4.10.0#1541

Merged
stephen-crawford merged 1 commit intoopensearch-project:mainfrom
cwperks:gha-fix-main
Aug 3, 2023
Merged

Force resolution of selenium-webdriver to 4.10.0#1541
stephen-crawford merged 1 commit intoopensearch-project:mainfrom
cwperks:gha-fix-main

Conversation

@cwperks
Copy link
Copy Markdown
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.

@cwperks
Force resolution of selenium-webdriver to 4.10.0
bdb2bde 
Signed-off-by: Craig Perkins <cwperx@amazon.com>
@codecov
Copy link
Copy Markdown

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
Copy Markdown
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
"html-entities": "1.3.1"
},
"resolutions": {
"selenium-webdriver": "4.10.0"
Copy link
Copy Markdown
Collaborator

@RyanL1997 RyanL1997 Aug 3, 2023

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
Copy Markdown
Member Author

@cwperks cwperks Aug 3, 2023

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
Copy Markdown
Member Author

@cwperks cwperks Aug 3, 2023

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 <cwperx@amazon.com>
(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 <cwperx@amazon.com>
(cherry picked from commit 95f316a)

Co-authored-by: Craig Perkins <cwperx@amazon.com>
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 <cwperx@amazon.com>
Signed-off-by: Sam <samuel.costa@eliatra.com>
@Desvelao Desvelao mentioned this pull request Oct 27, 2025
Merged
1 task
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

@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

+1 more reviewer

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

Reviewers whose approvals may not affect merge requirements

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