fix: do not override electron debug port if previously set

[warrensplayer]

• Closes Failed to connect to Chrome Debugging Protocol #26711

Additional details

PR #26573 added logic to assign a random available port for Electron to use as the debugging port to connect to CDP. This PR adds a check to make sure if a port has been passed in via setting an environment variable (i.e. ELECTRON_EXTRA_LAUNCH_ARGS=--remote-debugging-port=9222) as documented here (https://docs.cypress.io/api/plugins/browser-launch-api#Modify-Electron-app-switches) that it will not get overridden with a random port.

Steps to test

• Look at unit test added
• Run test with example repo in reported issue
  • Clone NeuraLegion/cypress-har-generator
  • cd to example directory
  • install Cypress 12.12.0 - npm i -D Cypress@12.12.0
  • install rest of dependencies - npm i
  • run tests - ELECTRON_EXTRA_LAUNCH_ARGS=--remote-debugging-port=9222 npm run test
  • view failure

     CypressError: `cy.task('recordHar')` failed with the following error:

> Failed to connect to Chrome Debugging Protocol

Possible reasons for failure:
  - Chrome not running in headless mode
  - Using Chrome version 58 or earlier
  - Inconsistent RDP configuration settings.

• View fix in this branch
  • in the example repo in the example directory, run npm run start to start the example server
  • switch to this branch in the Cypress project
  • cd packages/app
  • run ELECTRON_EXTRA_LAUNCH_ARGS=--remote-debugging-port=9222 yarn cypress:run-cypress-in-cypress node ../../scripts/cypress run --project {path_to}/cypress-har-generator/example and switch out path to the reproduction repo where it says path_to in this command
  • Tests should run successfully

How has the user experience changed?

N/A

PR Tasks

• Have tests been added/updated?
• [-] Has a PR for user-facing changes been opened in cypress-documentation?
• [-] Have API changes been updated in the type definitions?

[cypress]

30 flaky tests on run #48517 ↗︎

0

27937

1349

0

30

Details:

Merge branch 'develop' into stokes/26711_electron_debug_port
Project: cypress	Commit: 0b4f68584f
Status: Passed	Duration: 21:51 💡
Started: Jun 30, 2023 6:34 PM	Ended: Jun 30, 2023 6:56 PM

  commands/net_stubbing.cy.ts • 1 flaky test • 5x-driver-firefox

View Output Video

Test		Artifacts
network stubbing > intercepting request > can delay and throttle a StaticResponse		Output

  e2e/origin/commands/assertions.cy.ts • 1 flaky test • 5x-driver-firefox

View Output Video

Test		Artifacts
cy.origin assertions > #consoleProps > .should() and .and()		Output

  cypress/cypress.cy.js • 3 flaky tests • 5x-driver-firefox

View Output Video

Test		Artifacts
... > correctly returns currentRetry		Output
... > correctly returns currentRetry		Output
... > correctly returns currentRetry		Output

  create-from-component.cy.ts • 1 flaky test • app-e2e

View Output Video

Test		Artifacts
... > runs generated spec		Output Screenshots Video

  cypress-in-cypress.cy.ts • 1 flaky test • app-e2e

View Output Video

Test		Artifacts
Cypress in Cypress > scales the AUT correctly in e2e		Output Screenshots Video

The first 5 flaky specs are shown, see all 17 specs in Cypress Cloud.

_{This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.}

[mike-plummer]

Suggested change

	// if port was already set via passing from environment variable ELECTRON_EXTRA_LAUNCH_ARGS,
	// then just keep the supplied value
	if (app.commandLine.getSwitchValue('remote-debugging-port')) {
	return
	}
	const port = await getPort()
	// set up remote debugging port
	app.commandLine.appendSwitch('remote-debugging-port', String(port))
	return app.commandLine.appendSwitch
	// if port was already set via passing from environment variable ELECTRON_EXTRA_LAUNCH_ARGS,
	// then just keep the supplied value
	if (!app.commandLine.getSwitchValue('remote-debugging-port')) {
	const port = await getPort()
	// set up remote debugging port
	app.commandLine.appendSwitch('remote-debugging-port', String(port))
	}
	return app.commandLine.appendSwitch

Should we preserve the existing return value for either path?

[warrensplayer]

I was the one that added the return value when trying to debug my unit test. It is not used so I just removed it.

[lmiller1990]

Code seems fine. I followed the repro instructions but I did not encounter an error, though 🤔

[cypress-bot]

Released in 12.17.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v12.17.0, please open a new issue.