Skip to content

chore: add cdp connection to cy prompt #31806

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

Merged

Conversation

ryanthemanuel
Copy link
Collaborator

Additional details

cy.prompt will require a CDP connection to access A11y methods. This PR adds the ability to connect the CDP connection to the cloud-delivered cy.prompt

Steps to test

How has the user experience changed?

PR Tasks

@ryanthemanuel ryanthemanuel self-assigned this May 30, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds CDP support for cy.prompt by introducing a new connectCyPromptToBrowser workflow across the server, socket layers, project bootstrap, browser launchers, and tests.

  • Expose and wire connectCyPromptToBrowser in OpenProject, ProjectBase, and SocketBase
  • Extend each browser launcher (Chrome, Electron, Firefox, WebKit) and the core BrowserCriClient to clone and connect a separate CDP client for cy.prompt
  • Update TypeScript types, fixtures, and unit tests to cover the new CyPromptManager and CyPromptCDPClient integration

Reviewed Changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/server/lib/open_project.ts Added connectCyPromptToBrowser method
packages/server/lib/project-base.ts Hooked onCyPromptReady to connect prompt manager to browser
packages/server/lib/socket-base.ts Introduced onCyPromptReady socket handler
packages/server/lib/browsers/types.ts Extended BrowserLauncher type with connectCyPromptToBrowser
packages/server/lib/browsers/index.ts Dispatched connectCyPromptToBrowser to per-browser launcher
packages/server/lib/browsers/chrome.ts Implemented connectCyPromptToBrowser for Chrome
packages/server/lib/browsers/electron.ts Implemented connectCyPromptToBrowser for Electron
packages/server/lib/browsers/firefox.ts Stubbed unsupported prompt with an error
packages/server/lib/browsers/webkit.ts Stubbed unsupported prompt with an error
packages/server/lib/browsers/browser-cri-client.ts Extended BrowserCriClient to clone and notify cyPromptManager
packages/server/lib/cloud/cy-prompt/CyPromptManager.ts Added connectToBrowser API and CDP client handling
packages/server/test/unit/open_project_spec.js Added a new test context for connectCyPromptToBrowser
packages/server/test/unit/cloud/cy-prompt/CyPromptManager_spec.ts Updated tests for prompt manager, left TODOs for error assertions
packages/server/test/unit/browsers/webkit_spec.ts Added unsupported-prompt test for WebKit
packages/server/test/unit/browsers/firefox_spec.ts Added unsupported-prompt test for Firefox
packages/server/test/unit/browsers/electron_spec.js Added prompt-connection tests for Electron
packages/server/test/unit/browsers/chrome_spec.js Added prompt-connection tests for Chrome
packages/server/test/unit/browsers/browsers_spec.js Ensured top-level dispatcher routes prompt connection
packages/server/test/unit/browsers/browser-cri-client_spec.ts Added cyPromptManager to browser-cri-client tests
packages/server/test/support/fixtures/cloud/cy-prompt/test-cy-prompt.ts Updated test fixture to include a no-op connectToBrowser
Comments suppressed due to low confidence (4)

packages/server/test/unit/open_project_spec.js:275

  • This test calls connectCyPromptToBrowser but does not assert that browsers.connectCyPromptToBrowser was invoked; add an expectation such as expect(browsers.connectCyPromptToBrowser).to.be.calledWith(options).
await openProject.connectCyPromptToBrowser(options)

packages/server/test/unit/cloud/cy-prompt/CyPromptManager_spec.ts:56

  • Replace this TODO with a concrete assertion verifying that an error is reported or logged when initialization fails, e.g., checking an error callback or status change.
// TODO: test that the error is reported

packages/server/lib/browsers/firefox.ts:443

  • [nitpick] Inconsistent casing: consider capitalizing Firefox to match WebKit and maintain a consistent style in error messages.
throw new Error('CyPrompt is not yet supported in firefox.')

packages/server/lib/cloud/cy-prompt/CyPromptManager.ts:43

  • Changing isEssential from true to false may suppress critical errors for cy.prompt; consider retaining true to ensure upstream error handling remains active.
return this.invokeAsync('handleBackendRequest', { isEssential: false }, eventName, ...args)

Copy link

cypress bot commented May 30, 2025

cypress    Run #62841

Run Properties:  status check passed Passed #62841  •  git commit 335148973a: rework
Project cypress
Branch Review ryanm/chore/add-cdp-connection-to-cy-prompt
Run status status check passed Passed #62841
Run duration 19m 44s
Commit git commit 335148973a: rework
Committer Ryan Manuel
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 10
Tests that did not run due to a developer annotating a test with .skip  Pending 1232
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 32187
View all changes introduced in this branch ↗︎
UI Coverage  45.96%
  Untested elements 189  
  Tested elements 165  
Accessibility  92.74%
  Failed rules  3 critical   9 serious   2 moderate   2 minor
  Failed elements 698  

Copy link
Contributor

@AtofStryker AtofStryker left a comment

Choose a reason for hiding this comment

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

I guess it isn't entirely too clear to me why cy.prompt() needs it's own CDP connection. Any idea what the memory/CPU usage looks like with the newly cloned target?

@@ -1,5 +1,10 @@
describe('src/cy/commands/prompt', () => {
it('executes the prompt command', () => {
// TODO: test the error messages
Copy link
Contributor

Choose a reason for hiding this comment

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

is it possible to link this to an issue? Might be good to explain that cy.prompt() isn't supported in these browsers and that tests are coming to test the errors

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Contributor

@mschile mschile left a comment

Choose a reason for hiding this comment

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

Added a couple comments.

@ryanthemanuel ryanthemanuel merged commit ebba6e4 into feat/cy-prompt Jun 3, 2025
87 of 89 checks passed
@ryanthemanuel ryanthemanuel deleted the ryanm/chore/add-cdp-connection-to-cy-prompt branch June 3, 2025 15:03
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.

3 participants