feat: Support --browser cli option in Launchpad #18473

chrisbreiding · 2021-10-13T15:00:15Z

Closes Add support for cypress open --browser <browser> #18419

User facing changelog

cypress open --browser <browser> will automatically select the browser.

Additional details

Displays this when the browser can't be found or isn't supported:

cypress open --browser does-not-exist

PR Tasks

Have tests been added/updated?
Has the original issue or this PR been tagged with a release in ZenHub?
Has a PR for user-facing changes been opened in cypress-documentation?
Have API changes been updated in the type definitions?
Have new configuration options been added to the cypress.schema.json?

cypress-bot · 2021-10-13T15:00:18Z

Thanks for taking the time to open a PR!

Create a Draft Pull Request if your PR is not ready for review. Mark the PR as Ready for Review when you're ready for a Cypress team member to review the PR.
Become familiar with the Code Review Checklist for guidelines on coding standards and what needs to be done before a PR can be merged.

JessicaSachs

Tiny one-liner Vue change. Code looks good. I haven't manually tested this.

Still need:

Tests
Approval from design

packages/launchpad/src/setup/OpenBrowserList.vue

chrisbreiding · 2021-10-27T18:48:36Z

packages/graphql/src/schemaTypes/objectTypes/gql-Mutation.ts

@@ -232,10 +232,6 @@ export const mutation = mutationType({
    t.liveMutation('launchOpenProject', {
      description: 'Launches project from open_project global singleton',
      resolve: async (_, args, ctx) => {
-        if (!ctx.wizardData.chosenTestingType) {


This check is now done in ctx.actions.project.launchProject.

I think we probably just want to return null here - @tgriesser mentioned the convention when a mutation "fails" was just to return null if I recall correctly.

For Query fields "failing", you'd want to return null, I think for mutations that are called in an invalid way it probably makes sense to throw b/c you have better control of the order of execution of those.

So in this particular case, it should throw, right?

chrisbreiding · 2021-10-27T18:57:52Z

packages/server/lib/browsers/utils.ts

+  })
+}
+
+const isValidPathToBrowser = (str) => {


This is all moved here from browser/index.js so that it will play nicely with @packages/data-context, which is stricter about TypeScript, but none of it is fundamentally changed. The above function needed to be moved out of the object below so it can be referenced by these functions.

cypress · 2021-10-27T19:15:46Z

Test summary

18648 • 1 • 216 • 7 • 3

Run details


Project	cypress
Status	Failed
Commit	`eafe525`
Started	Nov 8, 2021 3:52 PM
Ended	Nov 8, 2021 4:05 PM
Duration	12:11 💡
OS	Linux Debian - 10.9
Browser	Multiple

View run in Cypress Dashboard ➡️

Failures

	cypress/integration/commands/window_spec.js		1
1	... > throws when passed an invalid orientation on a preset

Flakiness

	net_stubbing_spec.ts	2
1	network stubbing > waiting and aliasing > can timeout waiting on a single request using "alias.request"
2	network stubbing > waiting and aliasing > can timeout incrementally waiting on requests
	xhr_spec.js	1
1	... > no status when request isnt forced 404

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

cli/__snapshots__/cli_spec.js

chrisbreiding · 2021-11-03T18:09:24Z

Got approval from Ryan on the warning design.

tgriesser

Haven't looked into why the test is failing, but feel free to skip / add a TODO if you're having issues and I can address as part of #18776 which will look to cleanup the overall flow of testing CLI flags

ryanjwilke · 2021-11-04T15:39:14Z

packages/data-context/src/actions/AppActions.ts

+      browser = (await this.ctx._apis.appApi.ensureAndGetByNameOrPath(browserNameOrPath)) as FoundBrowser | undefined
+    } catch (err: unknown?) {
+      this.ctx.debug('Error getting browser by name or path (%s): %s', browserNameOrPath, err?.stack || err)
+      this.ctx.coreData.wizard.browserErrorMessage = `The browser '${browserNameOrPath}' was not found on your system or is not supported by Cypress. Choose a browser below.`


My only concern with this is that "Choose a browser below" feels pretty repetitive given the title of the page is essentially this as well.

ryanjwilke

I left one comment that would be nice to address before releasing this, but if others disagree, then there's no need to block this PR over that from my perspective.

tgriesser · 2021-11-08T14:16:49Z

Working on fixing the issue that's causing the failure here

tgriesser · 2021-11-08T16:05:58Z

I left one comment that would be nice to address before releasing this, but if others disagree, then there's no need to block this PR over that from my perspective.

@ryanjwilke Merging this PR to unblock separate work that's dependent on this landing to cleanup our testing

Probably worth making a note of your feedback and updating the wording here during our phase of testing/refining

* unified-desktop-gui: feat: Support --browser cli option in Launchpad (#18473) feat: Add typings for new devServer config (#18797) feat(app): wire up cy.screenshot (#18768) feat: create specs ui (#18483)

chrisbreiding added 2 commits October 13, 2021 10:57

update --browser description for cypress open

782d4ae

set active browser when specified by cli

c9dc139

chrisbreiding changed the base branch from develop to unified-desktop-gui October 13, 2021 15:00

chrisbreiding changed the title ~~Support --browser cli option in Launchpad~~ feat: Support --browser cli option in Launchpad Oct 13, 2021

chrisbreiding added 7 commits October 13, 2021 11:16

fix type

c62d44e

fix types

6b79733

fix tests

68217f4

Merge branch 'unified-desktop-gui' into issue-18419-open-browser-flag

755b2f7

Merge branch 'unified-desktop-gui' into issue-18419-open-browser-flag

9c3c584

display warning if browser can't be found

a17d85c

Merge branch 'unified-desktop-gui' into issue-18419-open-browser-flag

320c1db

JessicaSachs suggested changes Oct 26, 2021

View reviewed changes

packages/launchpad/src/setup/OpenBrowserList.vue Outdated Show resolved Hide resolved

chrisbreiding added 3 commits October 26, 2021 15:03

Merge branch 'unified-desktop-gui' into issue-18419-open-browser-flag

4e91b02

refactor and add component tests

025ea23

add e2e test

393892f

chrisbreiding marked this pull request as ready for review October 27, 2021 18:42

chrisbreiding requested a review from a team as a code owner October 27, 2021 18:42

chrisbreiding requested review from jennifer-shehane and removed request for a team October 27, 2021 18:42

Merge branch 'unified-desktop-gui' into issue-18419-open-browser-flag

777ace9

jennifer-shehane removed their request for review October 27, 2021 18:48

chrisbreiding commented Oct 27, 2021

View reviewed changes

fix

640ea53

chrisbreiding commented Oct 27, 2021

View reviewed changes

chrisbreiding requested review from JessicaSachs and lmiller1990 October 27, 2021 18:58

flotwig requested a review from BlueWinds October 28, 2021 15:34

Merge branch 'unified-desktop-gui' into issue-18419-open-browser-flag

83a79d5

move the condition

daca066

chrisbreiding requested review from lmiller1990 and BlueWinds November 2, 2021 13:34

Merge branch 'unified-desktop-gui' into issue-18419-open-browser-flag

497f582

emilyrohrbough reviewed Nov 2, 2021

View reviewed changes

cli/__snapshots__/cli_spec.js Show resolved Hide resolved

emilyrohrbough approved these changes Nov 2, 2021

View reviewed changes

chrisbreiding requested a review from ryanjwilke November 2, 2021 14:19

BlueWinds requested review from BlueWinds and removed request for BlueWinds November 2, 2021 16:01

chrisbreiding added 6 commits November 2, 2021 15:08

update warning design and placement

b245126

fix type

e6511b2

fix type issues

dd778e7

add/update tests

dfb9fac

Merge branch 'unified-desktop-gui' into issue-18419-open-browser-flag

132ace0

update config reference

c6fb400

chrisbreiding requested a review from tgriesser November 3, 2021 18:10

tgriesser approved these changes Nov 4, 2021

View reviewed changes

chrisbreiding added 2 commits November 4, 2021 09:36

Merge branch 'unified-desktop-gui' into issue-18419-open-browser-flag

c207820

fix test

8c56e56

ryanjwilke reviewed Nov 4, 2021

View reviewed changes

ryanjwilke approved these changes Nov 4, 2021

View reviewed changes

tgriesser added 2 commits November 8, 2021 10:45

Fix tests

633a246

Merge branch 'unified-desktop-gui' into issue-18419-open-browser-flag

eafe525

tgriesser merged commit a9902b2 into unified-desktop-gui Nov 8, 2021

tgriesser deleted the issue-18419-open-browser-flag branch November 8, 2021 16:06

cypress-bot bot mentioned this pull request Nov 8, 2021

Add support for cypress open --browser <browser> #18419

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Support --browser cli option in Launchpad #18473

feat: Support --browser cli option in Launchpad #18473

chrisbreiding commented Oct 13, 2021 •

edited

Loading

cypress-bot bot commented Oct 13, 2021

JessicaSachs left a comment

chrisbreiding Oct 27, 2021

lmiller1990 Nov 2, 2021 •

edited

Loading

chrisbreiding Nov 2, 2021

tgriesser Nov 2, 2021

chrisbreiding Nov 2, 2021

chrisbreiding Oct 27, 2021

cypress bot commented Oct 27, 2021 •

edited

Loading

chrisbreiding commented Nov 3, 2021

tgriesser left a comment

ryanjwilke Nov 4, 2021

ryanjwilke left a comment

tgriesser commented Nov 8, 2021

tgriesser commented Nov 8, 2021

feat: Support --browser cli option in Launchpad #18473

feat: Support --browser cli option in Launchpad #18473

Conversation

chrisbreiding commented Oct 13, 2021 • edited Loading

User facing changelog

Additional details

PR Tasks

cypress-bot bot commented Oct 13, 2021

JessicaSachs left a comment

Choose a reason for hiding this comment

chrisbreiding Oct 27, 2021

Choose a reason for hiding this comment

lmiller1990 Nov 2, 2021 • edited Loading

Choose a reason for hiding this comment

chrisbreiding Nov 2, 2021

Choose a reason for hiding this comment

tgriesser Nov 2, 2021

Choose a reason for hiding this comment

chrisbreiding Nov 2, 2021

Choose a reason for hiding this comment

chrisbreiding Oct 27, 2021

Choose a reason for hiding this comment

cypress bot commented Oct 27, 2021 • edited Loading

Test summary

Run details

Failures

Flakiness

chrisbreiding commented Nov 3, 2021

tgriesser left a comment

Choose a reason for hiding this comment

ryanjwilke Nov 4, 2021

Choose a reason for hiding this comment

ryanjwilke left a comment

Choose a reason for hiding this comment

tgriesser commented Nov 8, 2021

tgriesser commented Nov 8, 2021

chrisbreiding commented Oct 13, 2021 •

edited

Loading

lmiller1990 Nov 2, 2021 •

edited

Loading

cypress bot commented Oct 27, 2021 •

edited

Loading