Skip to content

Conversation

titusfortner
Copy link
Member

Description

  1. Don't execute Java ExecutableFinder() if a System Property has been defined
  2. Python log results even if error code isn't 0

Motivation and Context

Ensured logic the same between Java/Python/Ruby/.NET

@codecov-commenter
Copy link

Codecov Report

Patch coverage: 16.66% and no project coverage change.

Comparison is base (8ed2d9f) 57.22% compared to head (2d630da) 57.23%.

❗ Current head 2d630da differs from pull request most recent head b160b3e. Consider uploading reports for the commit b160b3e to get more accurate results

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@           Coverage Diff           @@
##            trunk   #12269   +/-   ##
=======================================
  Coverage   57.22%   57.23%           
=======================================
  Files          86       86           
  Lines        5480     5481    +1     
  Branches      226      226           
=======================================
+ Hits         3136     3137    +1     
  Misses       2118     2118           
  Partials      226      226           
Impacted Files Coverage Δ
py/selenium/webdriver/common/selenium_manager.py 63.49% <16.66%> (+0.58%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@diemol diemol left a comment

Choose a reason for hiding this comment

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

Does this mean we are letting Selenium Manager find the driver on the PATH in Java?

@titusfortner
Copy link
Member Author

No, not yet. The default parameter for getProperty always evaluates, even when there is a property, which is less performant and unnecessary. So very minor performance improvement; and now we just remove those 3 lines when we do let SM get from PATH.

@diemol diemol merged commit d05ab6f into trunk Jun 28, 2023
@diemol diemol deleted the sm_updates branch June 28, 2023 11:33
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.

3 participants