Add bsc_path helper + use bsc.exe for super-errors test #5471
Conversation
| @jchavarri @cristianoc This was merged even though it didn't build. It seems this PR broke Windows CI. |
| @cknitt sorry, i wrongly assumed windows ci build was not 100% reliable yet. Will fix. |
| // note existsSync test already ensure that it is a directory | ||
| try { | ||
| cp.execSync(`node input.js`, { cwd: testDir, encoding: "utf8" }); | ||
| cp.exec(`node input.js`, { cwd: testDir, encoding: "utf8" }); |
There was a problem hiding this comment.
If this is not sync anymore, doesn't that mean that success will be logged immediately and failure not caught?
There was a problem hiding this comment.
This was originally exec and @cristianoc changed it to sync in jchavarri@0fbf257. I understood this was part of the fix for tests hanging, that we could safely revert now.
If a callback is passed to exec then everything should work fine:
If a callback function is provided, it is called with the arguments (error, stdout, stderr). On success, error will be null. On error, error will be an instance of Error. The error.code property will be the exit code of the process. By convention, any exit code other than 0 indicates an error. error.signal will be the signal that terminated the process.
There was a problem hiding this comment.
Indeed the call-back needs to be added back, or this won't fail on failed tests.
There was a problem hiding this comment.
Added back in #5477.
3 participants
Fixes #5394.
Alternative to #5469.
bin_path.jswhere paths to bsc.exe and other binaries can be derived in a single placebsc.exepath directly in super-errors to avoid issues with hanging streamsbin_pathvalues across scripts.