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

`cypress run` does not log `stderr` from plugins #5765

Open
flotwig opened this issue Nov 21, 2019 · 8 comments · May be fixed by #5884
Open

`cypress run` does not log `stderr` from plugins #5765

flotwig opened this issue Nov 21, 2019 · 8 comments · May be fixed by #5884

Comments

@flotwig
Copy link
Member

@flotwig flotwig commented Nov 21, 2019

Current behavior:

Run this existing spec that has a plugin which logs to process.stderr:

npm run cypress:run -- --project ./packages/server/test/support/fixtures/projects/system-node
npm run cypress:open -- --project ./packages/server/test/support/fixtures/projects/system-node

module.exports = (onFn, config) => {
process.stderr.write('Plugin Loaded\n')
process.stderr.write(`Plugin Node version: ${process.versions.node}\n`)
process.stderr.write(`Plugin Electron version: ${process.versions.electron}\n`)
}

It will log in cypress open, but not cypress run.

Desired behavior:

The pluginsfile should log either way.

@flotwig

This comment has been minimized.

Copy link
Member Author

@flotwig flotwig commented Nov 25, 2019

The culprit is that this code path will cause the onStderrData callback to always return falsey:

if (linuxWithDisplayEnv) {
_.extend(overrides, {
electronLogging: true,
onStderrData (str) {
// if we receive a broken pipe anywhere
// then we know that's why cypress exited early
if (util.isBrokenGtkDisplay(str)) {
brokenGtkDisplay = true
}
// we should attempt to always slurp up
// the stderr logs unless we've explicitly
// enabled the electron debug logging
if (!debugElectron.enabled) {
return false
}
},
})
}

I believe this was a mistake, as line 252 returns false, but there is no "default case" that returns true after that.

If you add "return true", once again, output will contain Electron debug information (#4255). But we can just filter that out or not set ELECTRON_ENABLE_LOGGING at any point.

@santoshyadav198613

This comment has been minimized.

Copy link
Contributor

@santoshyadav198613 santoshyadav198613 commented Dec 4, 2019

HI @jennifer-shehane ,
I would like to take this issue.

@flotwig

This comment has been minimized.

Copy link
Member Author

@flotwig flotwig commented Dec 4, 2019

@santoshyadav198613 Feel free to pick this issue up, ping me if you have any questions. I think changing that onStderrData callback to return true will be a good starting point.

@santoshyadav198613

This comment has been minimized.

Copy link
Contributor

@santoshyadav198613 santoshyadav198613 commented Dec 5, 2019

Hi @flotwig ,
Are we looking for this to be logged?
image

santoshyadav198613 added a commit to santoshyadav198613/cypress that referenced this issue Dec 5, 2019
santoshyadav198613 added a commit to santoshyadav198613/cypress that referenced this issue Dec 5, 2019
santoshyadav198613 added a commit to santoshyadav198613/cypress that referenced this issue Dec 5, 2019
santoshyadav198613 added a commit to santoshyadav198613/cypress that referenced this issue Dec 5, 2019
@santoshyadav198613 santoshyadav198613 linked a pull request that will close this issue Dec 5, 2019
1 of 5 tasks complete
@santoshyadav198613

This comment has been minimized.

Copy link
Contributor

@santoshyadav198613 santoshyadav198613 commented Dec 5, 2019

Hi @flotwig ,
PR is open now.

@flotwig

This comment has been minimized.

Copy link
Member Author

@flotwig flotwig commented Dec 5, 2019

Hi @flotwig ,
Are we looking for this to be logged?
image

Yep, that's great!

@santoshyadav198613

This comment has been minimized.

Copy link
Contributor

@santoshyadav198613 santoshyadav198613 commented Dec 5, 2019

Hi @flotwig,
Cool just need some help on Testing , mentioned the error which I am getting in PR.

santoshyadav198613 added a commit to santoshyadav198613/cypress that referenced this issue Dec 5, 2019
@justin335

This comment has been minimized.

Copy link

@justin335 justin335 commented Dec 13, 2019

Nice

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
You can’t perform that action at this time.