-
Notifications
You must be signed in to change notification settings - Fork 3.3k
fix: run-all-specs opens in new tab rather than new browser #25074
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
Conversation
Thanks for taking the time to open a PR!
|
@@ -1255,7 +1255,7 @@ type Mutation { | |||
internal_clearProjectPreferencesCache(projectTitle: String!): Boolean | |||
|
|||
"""Launches project from open_project global singleton""" | |||
launchOpenProject(specPath: String): CurrentProject | |||
launchOpenProject(shouldLaunchNewTab: Boolean, specPath: String): CurrentProject |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something is strange here - the order of the arguments is the opposite on the client mutation? https://github.com/cypress-io/cypress/pull/25074/files#diff-4efefe4d3a02c250b8acd5d08fb87caf32cf883ccff71ea7965d0e6c0448fe68R25
Also, I think it makes sense for the specPath to be first arg, and shouldLaunchNewTab to be second - it follows the usual convention of putting the options as the second argument.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am surprised this works at all - GraphQL should be erroring, the types don't match.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is just sorted alphabetically (this is generated). Look at:
cloudProjectCreate(campaign: String, ciProviders: [String!], cohort: String, medium: String, name: String!, orgId: ID!, public: Boolean!, source: String): CloudProject
Order doesn't matter since all the args resolve to an args
object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I see. I didn't realize that (and it's quite confusing - the order generally DOES matter in GraphQL, I guess not if you are using Nexus).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested it out, works great. Curious about the order of the GraphQL mutation arguments, though.
@@ -1255,7 +1255,7 @@ type Mutation { | |||
internal_clearProjectPreferencesCache(projectTitle: String!): Boolean | |||
|
|||
"""Launches project from open_project global singleton""" | |||
launchOpenProject(specPath: String): CurrentProject | |||
launchOpenProject(shouldLaunchNewTab: Boolean, specPath: String): CurrentProject |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am surprised this works at all - GraphQL should be erroring, the types don't match.
@@ -1255,7 +1255,7 @@ type Mutation { | |||
internal_clearProjectPreferencesCache(projectTitle: String!): Boolean | |||
|
|||
"""Launches project from open_project global singleton""" | |||
launchOpenProject(specPath: String): CurrentProject | |||
launchOpenProject(shouldLaunchNewTab: Boolean, specPath: String): CurrentProject |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I see. I didn't realize that (and it's quite confusing - the order generally DOES matter in GraphQL, I guess not if you are using Nexus).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Oh, one last thing we should check @ZachJW34 - are videos getting recorded correctly? How does this manifest in Cypress Cloud? Edit: actually doesn't matter for Cloud, this is open mode only - still relevant locally though, since you can record videos locally (although not as common/useful, worth knowing if/how this works). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested this and it works well 👍🏻
@lmiller1990 you cannot record videos when in open mode. Since this feature is only related to open-mode, this should have no affect on video recording. |
Released in This comment thread has been locked. If you are still experiencing this issue after upgrading to |
User facing changelog
Run All Specs will open in a new tab rather than close and reopen the browser
Additional details
Following the same run.ts logic for retaining the browser and switching out the tab with a fresh one.
Steps to test
Choose a project that has Cypress setup and add
e2e.experimentalRunAllSpecs = true
to the config. Check out this branch, runyarn
and theyarn cypress:open
. Open up a browser and verify that, after clicking "Run all specs", the browser isn't closed and the current tab is replaced with a new one.Electron still spawns a new window since there is no "New Tab" in Electron.
Not sure the best way to test this change, since any tests around
launchProject
require opening and closing browsers.How has the user experience changed?
Screen.Recording.2022-12-08.at.11.58.07.AM.mov
PR Tasks
cypress-documentation
?type definitions
?