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

feat: Set up cypress in cypress #19602

Merged
merged 77 commits into from
Jan 14, 2022

Conversation

ryanthemanuel
Copy link
Collaborator

@ryanthemanuel ryanthemanuel commented Jan 6, 2022

User facing changelog

N/A

Additional details

From the list of items on the parent issue, the ones done with this PR are crossed off. The ones not will be handled by separate PRs, but with this change it should unblock people from writing tests

Namespacing

  • General routes need to be namespaced
    • We will ensure that we are both utilizing the namespace on the client and factoring in the namespace in our route handling on the server
    • Currently the config is being retrieved via a call to the server. This will prevent the child Cypress instance config from being able to be loaded properly on the client since it doesn't know to namespace the API call yet. We will fix this by rendering the cypress config to the initial page server side in a global variable that can then be utilized client side to access the config.
  • Websockets need to be namespaced
    • We will ensure that we are using the config.socketIoRoute as the path wherever we are creating a websocket
    • The proxied websockets from parent cypress to child cypress do not create an initial http connect event and thus the socket is not added to the allowed list for the child app server. Solution for this is TBD. For now, we will pass a header from parent to child signifying that this is a safe request. The child will only honor this header if we are in Cypress in Cypress mode
  • Graphql requests need to be namespaced

Other Problems

  • Connecting to an already existing browser
    • Since we already have a browser running when the child cypress server starts, we will need to set up a remote connection. We will do this by using CDPAutomation to attach to the existing browser instead of opening a new browser instance when we are in cypress in cypress. In order to facilitate this we are going to launch cypress in cypress tests with the environment variable set that specifies the remote debugging port. Also we are focused only on chrome

How has the user experience changed?

N/A

PR Tasks

  • Have tests been added/updated?
  • [na] Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
  • [na] Has a PR for user-facing changes been opened in cypress-documentation?
  • [na] Have API changes been updated in the type definitions?
  • [na] Have new configuration options been added to the cypress.schema.json?

Sorry, something went wrong.

Verified

This commit was signed with the committer’s verified signature.
ryanthemanuel Ryan Manuel

Verified

This commit was signed with the committer’s verified signature.
ryanthemanuel Ryan Manuel

Verified

This commit was signed with the committer’s verified signature.
ryanthemanuel Ryan Manuel

Verified

This commit was signed with the committer’s verified signature.
ryanthemanuel Ryan Manuel

Verified

This commit was signed with the committer’s verified signature.
ryanthemanuel Ryan Manuel
…on and ensure proxied websockets with cypress in cypress are allowed

Verified

This commit was signed with the committer’s verified signature.
ryanthemanuel Ryan Manuel
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jan 6, 2022

Thanks for taking the time to open a PR!

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@ryanthemanuel ryanthemanuel changed the title Set up troubleshooting cypress in cypress feat: Set up cypress in cypress Jan 6, 2022

Verified

This commit was signed with the committer’s verified signature.
ryanthemanuel Ryan Manuel

Verified

This commit was signed with the committer’s verified signature.
ryanthemanuel Ryan Manuel
@cypress
Copy link

cypress bot commented Jan 6, 2022



Test summary

18554 0 219 0Flakiness 1


Run details

Project cypress
Status Passed
Commit be38c7a
Started Jan 14, 2022 10:18 PM
Ended Jan 14, 2022 10:32 PM
Duration 13:24 💡
OS Linux Debian - 10.10
Browser Multiple

View run in Cypress Dashboard ➡️


Flakiness

cypress/e2e/commands/net_stubbing.cy.ts Flakiness
1 network stubbing > waiting and aliasing > can timeout waiting on a single request using "alias.request"

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

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was signed with the committer’s verified signature.
ryanthemanuel Ryan Manuel

Verified

This commit was signed with the committer’s verified signature.
ryanthemanuel Ryan Manuel

Verified

This commit was signed with the committer’s verified signature.
ryanthemanuel Ryan Manuel

Verified

This commit was signed with the committer’s verified signature.
ryanthemanuel Ryan Manuel
…ing it is a global

Verified

This commit was signed with the committer’s verified signature.
ryanthemanuel Ryan Manuel

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was signed with the committer’s verified signature.
ryanthemanuel Ryan Manuel

Verified

This commit was signed with the committer’s verified signature.
ryanthemanuel Ryan Manuel

Verified

This commit was signed with the committer’s verified signature.
ryanthemanuel Ryan Manuel

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was signed with the committer’s verified signature.
ryanthemanuel Ryan Manuel
…king

Verified

This commit was signed with the committer’s verified signature.
ryanthemanuel Ryan Manuel

Verified

This commit was signed with the committer’s verified signature.
ryanthemanuel Ryan Manuel

Verified

This commit was signed with the committer’s verified signature.
ryanthemanuel Ryan Manuel

Verified

This commit was signed with the committer’s verified signature.
ryanthemanuel Ryan Manuel

Verified

This commit was signed with the committer’s verified signature.
ryanthemanuel Ryan Manuel

Verified

This commit was signed with the committer’s verified signature.
ryanthemanuel Ryan Manuel

Verified

This commit was signed with the committer’s verified signature.
ryanthemanuel Ryan Manuel

Verified

This commit was signed with the committer’s verified signature.
ryanthemanuel Ryan Manuel
@ryanthemanuel
Copy link
Collaborator Author

@lmiller1990 regarding #19602 (comment) I think as I was going through I was just ensuring that we are being consistent everywhere with the namespace. Good to know that package is going away though.

Verified

This commit was signed with the committer’s verified signature.
ryanthemanuel Ryan Manuel

Verified

This commit was signed with the committer’s verified signature.
ryanthemanuel Ryan Manuel
Comment on lines 11 to 15
"cypress:run-cypress-in-cypress": "cross-env HTTP_PROXY_TARGET_HOST=http://localhost:4455 CYPRESS_REMOTE_DEBUGGING_PORT=6666 TZ=America/New_York",
"cypress:launch": "yarn cypress:run-cypress-in-cypress gulp open --project ${PWD}",
"cypress:open": "yarn cypress:run-cypress-in-cypress gulp open --project ${PWD}",
"cypress:run:ct": "yarn cypress:run-cypress-in-cypress node ../../scripts/cypress run --component --project ${PWD}",
"cypress:run:e2e": "yarn cypress:run-cypress-in-cypress node ../../scripts/cypress run --project ${PWD}",
Copy link
Member

Choose a reason for hiding this comment

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

My only concern was that if we set HTTP_PROXY_TARGET_HOST / CYPRESS_REMOTE_DEBUGGING_PORT here, we might miss it if Cypress is started from the root of the project, etc.

Can we set these as additional "internal" config values that we can set on the packages/app/cypress.config.ts? It looks like they're only accessed when the browser is opened, so that should be fine since the config is sourced prior to that. @brian-mann thoughts?

Copy link
Member

@brian-mann brian-mann Jan 14, 2022

Choose a reason for hiding this comment

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

I would prefer not to since they're not like... official config values / flags, and may end up getting removed anyway

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 not sure what CYPRESS_REMOTE_DEBUGGING_PORT does anyway... is this the port like in --inspect-brk=[port] ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@brian-mann CYPRESS_REMOTE_DEBUGGING_PORT is the port that is used to launch the chrome debugger. I need a way to ensure that the parent and child are using the same port.

Copy link
Member

Choose a reason for hiding this comment

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

We have a lot of these already:

cypress/cli/types/cypress.d.ts

Lines 2896 to 2923 in 29f5c64

// Internal or Unlisted at server/lib/config_options
autoOpen: boolean
browserUrl: string
clientRoute: string
configFile: string
cypressEnv: string
devServerPublicPathRoute: string
isNewProject: boolean
isTextTerminal: boolean
morgan: boolean
namespace: string
parentTestsFolder: string
parentTestsFolderDisplay: string
projectName: string
projectRoot: string
proxyUrl: string
remote: RemoteState
report: boolean
reporterRoute: string
reporterUrl: string
socketId: null | string
socketIoCookie: string
socketIoRoute: string
spec: Cypress['spec'] | null
specs: Array<Cypress['spec']>
xhrRoute: string
xhrUrl: string
}
- what alternative would you suggest?

Copy link
Collaborator Author

@ryanthemanuel ryanthemanuel Jan 14, 2022

Choose a reason for hiding this comment

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

@tgriesser Setting these on packages/app/cypress.config.ts doesn't seem to be early enough for parent cypress

Edit: The above comment is with respect to setting an environment variable in the cypress config file. I imagine if we add a first class internal config this will work.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was signed with the committer’s verified signature.
ryanthemanuel Ryan Manuel

Verified

This commit was signed with the committer’s verified signature.
ryanthemanuel Ryan Manuel

Verified

This commit was signed with the committer’s verified signature.
ryanthemanuel Ryan Manuel

Verified

This commit was signed with the committer’s verified signature.
ryanthemanuel Ryan Manuel
This reverts commit 8a25ab2.

Verified

This commit was signed with the committer’s verified signature.
ryanthemanuel Ryan Manuel

Verified

This commit was signed with the committer’s verified signature.
ryanthemanuel Ryan Manuel

Verified

This commit was signed with the committer’s verified signature.
ryanthemanuel Ryan Manuel
@ryanthemanuel ryanthemanuel requested a review from flotwig January 14, 2022 21:45

Verified

This commit was signed with the committer’s verified signature.
ryanthemanuel Ryan Manuel

Verified

This commit was signed with the committer’s verified signature.
ryanthemanuel Ryan Manuel

Verified

This commit was signed with the committer’s verified signature.
ryanthemanuel Ryan Manuel
@tgriesser tgriesser self-requested a review January 14, 2022 23:00
Copy link
Member

@tgriesser tgriesser left a comment

Choose a reason for hiding this comment

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

I think this looks great - as pointed out there's a few things we might be able to improve upon around the sockets & env variables needing to be passed by the CLI, but overall I think it's good to merging and iterate from here

@ryanthemanuel ryanthemanuel merged commit ed51bcb into 10.0-release Jan 14, 2022
@ryanthemanuel ryanthemanuel deleted the cypress-in-cypress-troubleshooting branch January 14, 2022 23:07
tgriesser added a commit that referenced this pull request Jan 19, 2022
* 10.0-release:
  feat: add cy.mount() runner error (#19145)
  docs: testing strategy/styleguide (#19639)
  fix: remove optional chaining from js files (#19702)
  feat: Set up cypress in cypress (#19602)
  feat: use electron folder select (#19692)
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.

None yet

6 participants