search

callstack / reassure

Performance testing companion for React and React Native
https://callstack.github.io/reassure/
MIT License
1.22k stars 26 forks source link

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

Open zacharyfmarion opened 1 year ago

zacharyfmarion commented 1 year ago

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.

thymikee commented 1 year ago

I'm good with that. @mdjastrzebski wdyt?

Xiltyn commented 1 year ago

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.

mdjastrzebski commented 4 months ago

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 commented 3 weeks ago

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