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

Launch browsers by path: --browser /path/to/browser #3337

Merged
merged 76 commits into from
Feb 16, 2019

Conversation

flotwig
Copy link
Contributor

@flotwig flotwig commented Feb 6, 2019

Closes #1026

Depends on merging #3225

Todo:

  • write tests for the feature
  • clean up errors in open mode
  • figure out why displayName isn't changing

@flotwig flotwig mentioned this pull request Feb 6, 2019
14 tasks
@flotwig
Copy link
Contributor Author

flotwig commented Feb 7, 2019

cypress open -P ../example-kitchensink --browser /usr/local/bin/chromium:

image

image

cli/__snapshots__/cli_spec.js Outdated Show resolved Hide resolved
packages/server/test/e2e/2_browser_path_spec.coffee Outdated Show resolved Hide resolved
packages/server/lib/gui/events.coffee Outdated Show resolved Hide resolved
packages/server/test/support/helpers/e2e.coffee Outdated Show resolved Hide resolved
packages/server/test/support/helpers/e2e.coffee Outdated Show resolved Hide resolved
@flotwig
Copy link
Contributor Author

flotwig commented Feb 14, 2019

@brian-mann Updated the error messages like we talked about. @lilaconlee's changes make details into a cool expando which is much more understandable for the user:

peek 2019-02-14 15-23

CLI error looks better now too:

image

@brian-mann
Copy link
Member

Love the error, it does look much better. Look what you did @lilaconlee :P

expectedExitCode: 1
})

it "works with an installed browser path", ->
Copy link
Member

Choose a reason for hiding this comment

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

Here's a slightly more condensed version of this taking advantage of bluebird...

launcher.detect()
.then (browsers) =>
  _.find(browsers, { family: 'chrome' })
.tap (browser) =>
  if !browser
    throw new Error("A 'chrome' family browser must be installed for this test. None were found.")
.get('path')
## turn binary browser names ("google-chrome") into their absolute paths
## so that server recognizes them as a path, not as a browser name
.then(absPath)
.then (foundPath) =>
  e2e.exec(@, {
    project: Fixtures.projectPath("e2e")
    spec: "simple_spec.coffee"
    browser: foundPath
    snapshot: true
    expectedExitCode: 0
  })

Fixtures = require("../support/helpers/fixtures")
launcher = require("@packages/launcher")

absPath = (pathStr) ->
Copy link
Member

Choose a reason for hiding this comment

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

I'm going to approve this PR - but this feels like this is something that should be part of the found browsers objects in launcher.

It looks like we have a path property... but apparently that only returns the binary executable and not the actual path to it?

@brian-mann brian-mann merged commit ef9b8f7 into develop Feb 16, 2019
@flotwig flotwig mentioned this pull request Feb 25, 2019
@flotwig flotwig deleted the issue-1026-browser-by-path branch January 24, 2022 18:17
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