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

Look for Selenium Manager in path defined by Environment Variable #12752

Merged
merged 5 commits into from
Sep 24, 2023

Conversation

titusfortner
Copy link
Member

Description

  • each of the bindings look for SE_MANAGER_PATH environment variable for Selenium Manager first before looking in the standard location

Motivation and Context

@titusfortner
Copy link
Member Author

I guess I should say, this doesn't *have to be an alternative. We could do both.

I think if we want to get 4.13 out soon, this one is sufficient for that need instead of waiting to implement #11359 in all the languages.

@codecov-commenter
Copy link

codecov-commenter commented Sep 15, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.03% ⚠️

Comparison is base (44ac51f) 56.53% compared to head (bca789f) 56.51%.
Report is 1 commits behind head on trunk.

❗ Current head bca789f differs from pull request most recent head 68c5eb2. Consider uploading reports for the commit 68c5eb2 to get more accurate results

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##            trunk   #12752      +/-   ##
==========================================
- Coverage   56.53%   56.51%   -0.03%     
==========================================
  Files          86       86              
  Lines        5253     5255       +2     
  Branches      187      187              
==========================================
  Hits         2970     2970              
- Misses       2096     2098       +2     
  Partials      187      187              
Files Changed Coverage Δ
py/selenium/webdriver/common/selenium_manager.py 57.89% <100.00%> (-1.57%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@titusfortner titusfortner changed the title Se mgr path Look for Selenium Manager in path defined by Environment Variable Sep 15, 2023
@titusfortner
Copy link
Member Author

Hmm, but looks like I broke some tests. 😂

@titusfortner
Copy link
Member Author

Hah, busted for not using Java 8 syntax. Hopefully last time that ever happens.

@titusfortner
Copy link
Member Author

Ok, so I made a few important changes.

  1. It expects the filename to be part of environment variable, not just the directory.
  2. It defaults to using linux even if code doesn't know what it is; (this matches Java implementation)

Copy link
Member

@nvborisenko nvborisenko left a comment

Choose a reason for hiding this comment

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

.net code reads env variable twice, can be improved.

…ronment variable

This allows people to build or download the binary as necessary and place it in any directory
It may also be helpful for package managers or people who have permissions issues in directories
@titusfortner titusfortner merged commit e5ce6fd into trunk Sep 24, 2023
56 of 60 checks passed
@titusfortner titusfortner deleted the se_mgr_path branch September 24, 2023 23:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants