Skip to content

Get job id only if sauce environment variables are set#35007

Merged
phil-davis merged 1 commit into
masterfrom
skshetry/job-id-on-sauce
Apr 12, 2019
Merged

Get job id only if sauce environment variables are set#35007
phil-davis merged 1 commit into
masterfrom
skshetry/job-id-on-sauce

Conversation

@skshetry

Copy link
Copy Markdown
Contributor

The getWebDriverSession is not part of the Mink\Driver\DriverInterface, but is provided
by the Selenium2Driver. So, as Sauce uses selenium grid, this will only be used
when Sauce environment variables are set (other drivers do not have this).

I noticed this when trying to use ChromeDriver instead of Selenium2Driver. We currently only support selenium, though.

Description

Related Issue

Motivation and Context

Tried to make it work on ChromeDriver.

How Has This Been Tested?

Not Tested. Just moved the line inside the conditional check.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

Open tasks:

  • Backport (if applicable set "backport-request" label and remove when the backport was done)

The `getWebDriverSession` is not part of the `Mink\Driver\DriverInterface`, but is provided
by the `Selenium2Driver`. So, as Sauce uses selenium grid, this will only be used
when Sauce environment variables are set (other drivers do not have this).
@skshetry

skshetry commented Apr 12, 2019

Copy link
Copy Markdown
Contributor Author

When researching this, I found getWebDriverSessionId already implemented by the Selenium2Driver.

https://github.com/minkphp/MinkSelenium2Driver/blob/8684ee4e634db7abda9039ea53545f86fc1e105a/src/Selenium2Driver.php#L1000

But, our getSessionId works fine for now.

@codecov

codecov Bot commented Apr 12, 2019

Copy link
Copy Markdown

Codecov Report

Merging #35007 into master will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #35007      +/-   ##
============================================
+ Coverage     65.37%   65.37%   +<.01%     
  Complexity    18622    18622              
============================================
  Files          1215     1215              
  Lines         70510    70510              
  Branches       1295     1295              
============================================
+ Hits          46095    46096       +1     
+ Misses        24041    24040       -1     
  Partials        374      374
Flag Coverage Δ Complexity Δ
#javascript 52.85% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 66.82% <ø> (ø) 18622 <ø> (ø) ⬇️
Impacted Files Coverage Δ Complexity Δ
lib/private/Server.php 86.68% <0%> (+0.11%) 253% <0%> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1fe0c83...a1488d5. Read the comment docs.

@skshetry skshetry marked this pull request as ready for review April 12, 2019 05:11
@skshetry skshetry self-assigned this Apr 12, 2019
@phil-davis

Copy link
Copy Markdown
Contributor

Backport stable10 #35008

@lock lock Bot locked as resolved and limited conversation to collaborators Apr 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants