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

Unify browserName given through different sources #390

Merged
merged 1 commit into from
Jun 26, 2024

Conversation

aik099
Copy link
Member

@aik099 aik099 commented Mar 19, 2024

The browser name can be specified through these places:

  • the $browserName driver constructor argument;
  • the $desiredCapabilities['browserName'] key of the desired capabilities (either given upon driver construction or later on).

Both browser names have no relation to each other which can lead to inconsistent driver configuration, e.g. $this->browserName set to the chrome, but in $this->desiredCapabilties['browserName'] set to the firefox.

P.S.
Nobody was noticing any bugs, because the \WebDriver\WebDriver::session method (a dependency of this project) has a polyfill code, that does the same thing as this PR.

The above-mentioned polyfill code is removed in 2.x releases of the WebDriver, which allowed us to spot this problem.

Copy link

codecov bot commented Mar 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.21%. Comparing base (a3a5370) to head (a6cfcfa).

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #390      +/-   ##
============================================
+ Coverage     90.19%   90.21%   +0.02%     
  Complexity      168      168              
============================================
  Files             1        1              
  Lines           469      470       +1     
============================================
+ Hits            423      424       +1     
  Misses           46       46              

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

@aik099 aik099 requested a review from stof March 19, 2024 10:05
@stof stof merged commit a56db27 into minkphp:master Jun 26, 2024
11 checks passed
@aik099 aik099 deleted the desired-capabilities-fix branch June 28, 2024 09:40
@aik099
Copy link
Member Author

aik099 commented Jun 28, 2024

Thank you, @stof .

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.

2 participants