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

remote-debugging-port cannot be set using ELECTRON_EXTRA_LAUNCH_ARGS #7994

Closed
dg-nvm opened this issue Jul 15, 2020 · 5 comments · Fixed by #8001
Closed

remote-debugging-port cannot be set using ELECTRON_EXTRA_LAUNCH_ARGS #7994

dg-nvm opened this issue Jul 15, 2020 · 5 comments · Fixed by #8001

Comments

@dg-nvm
Copy link

dg-nvm commented Jul 15, 2020

Current behavior:

When set ELECTRON_EXTRA_LAUNCH_ARGS=--remote-debugging-port 40500 is used then nothing happens

BUT

When I change environment.js to use appendSwitch instead, it works:

app.commandLine.appendSwitch('remote-debugging-port', '40500')

Desired behavior:

I can set option via environment variable

Test code to reproduce

set ELECTRON_EXTRA_LAUNCH_ARGS=--remote-debugging-port 40500
cypress open

Cypress should output:
DevTools listening on ws://127.0.0.1:40500/devtools/browser/<UUID>

Versions

Windows 10, Cypress 4.10.0, using builtin Electron in Headed mode

@jennifer-shehane
Copy link
Member

Yeah, I don't think we are handling key value pairs correctly in our ELECTRON_EXTRA_LAUNCH_ARGS here. Electron says we need to call the argument like app.commandLine.appendSwitch('remote-debugging-port', '8315'), but I don't see us actually pulling out the value from the key in order to pass these as 2 arguments to appendSwitch. https://www.electronjs.org/docs/api/command-line-switches

The code that handles this is here: https://github.com/cypress-io/cypress/blob/develop/packages/server/lib/environment.js#L71:L71

The test shows it's being called as --remote-debugging-port=40500.

I don't think that appendArgument actually supports key value pairs, the Electron docs even suggest using appendSwitch here instead. https://www.electronjs.org/docs/api/command-line#commandlineappendargumentvalue

@cypress-bot cypress-bot bot added the stage: ready for work The issue is reproducible and in scope label Jul 16, 2020
@cypress-bot cypress-bot bot added stage: work in progress and removed stage: ready for work The issue is reproducible and in scope labels Jul 16, 2020
@jennifer-shehane
Copy link
Member

Opened a PR here #8001

@jennifer-shehane jennifer-shehane self-assigned this Jul 16, 2020
@cypress-bot cypress-bot bot added stage: needs review The PR code is done & tested, needs review and removed stage: work in progress labels Jul 16, 2020
@dg-nvm
Copy link
Author

dg-nvm commented Jul 16, 2020

@jennifer-shehane thank you Jennifer for fast reaction. These two differs greatly internally and also they mention added cmdline will be added before switches.

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jul 16, 2020

The code for this is done in cypress-io/cypress#8001, but has yet to be released.
We'll update this issue and reference the changelog when it's released.

@cypress-bot cypress-bot bot added stage: pending release and removed stage: needs review The PR code is done & tested, needs review labels Jul 16, 2020
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jul 21, 2020

Released in 4.11.0.

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

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Jul 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants