reassure icon indicating copy to clipboard operation
reassure copied to clipboard

[FEATURE] Add ability to pass --inspect flag to node to debug test execution

Open zacharyfmarion opened this issue 3 years ago • 3 comments

Is your feature request related to a problem? Please describe.

When tests are unstable locally, it is hard to figure out why this would be happening.

Describe the solution you'd like A clear and concise description of what you want to happen.

When we have run into issues with unstable or long running tests it has been useful to pass an --inspect flag to the node child process (just by editing the lib commonjs folder in reassure-cli). It would be nice if we could just run yarn reassure --baseline --inspect and have that flag automatically added. Or we could use the --inspect-brk option, or both. Assuming (but have not confirmed) this has the potential to affect the stability of test results, so we could have a big warning saying something along the lines of "You are running node with the --inspect flag, this should not be used in CI to measure performance as it can result in greater render variance".

I'd be happy to put up a PR with the change and some documentation.

Describe alternatives you've considered A clear and concise description of any alternative solutions or features you've considered.

Editing node modules is a bit of a pain but it is fine if we don't want to support this in the library.

Additional context Add any other context or screenshots about the feature request here.

zacharyfmarion avatar Dec 18 '22 18:12 zacharyfmarion

I'm good with that. @mdjastrzebski wdyt?

thymikee avatar Dec 19 '22 08:12 thymikee

I'm ok with that as long as we clearly describe that running with the flag is for debugging the process only. I would like to see a prompt in the command line after starting a script with the flag asking the user explicitly You're running the script with XYZ which may impact your measurements. Are you sure you want to continue? (y/N) or something along these lines.

A proper note in the docs would also be required as mentioned :)

I'm fine with that, @mdjastrzebski?

I would say it'd be best to wait until the CLI init command PR is merged because it adds things like enquirer and also unifies some stuff in the CLI package.

Xiltyn avatar Feb 07 '23 10:02 Xiltyn

Seems like an interesting but pretty niche feature, that would require users to be familiar with nodes debugger, etc. We could support it if didn't affect regular users, but could be enabled e.g. by CLI flag.

mdjastrzebski avatar Mar 08 '24 12:03 mdjastrzebski

Based on other user requests, I am considering passing all (not consumed) flags to underlying Jest binary. Seems like it could solve your case.

mdjastrzebski avatar Jul 03 '24 15:07 mdjastrzebski

@mdjastrzebski What do you think about adding '--node-flags' to indicate that all subsequent flags should be passed to the underlying Node process? We could similarly handle Jest flags 🤔

V3RON avatar Aug 08 '24 06:08 V3RON