feat: add test for onLine throw error #4542
Conversation
|
✨ Coder.com for PR #4542 deployed! It will be updated on every commit.
|
Codecov Report
@@ Coverage Diff @@
## main #4542 +/- ##
==========================================
+ Coverage 65.38% 65.44% +0.06%
==========================================
Files 30 30
Lines 1664 1664
Branches 335 335
==========================================
+ Hits 1088 1089 +1
+ Misses 488 487 -1
Partials 88 88
Continue to review full report at Codecov.
|
test/unit/node/util.test.ts
Outdated
| // @ts-expect-error TypeScript knows proc.stdout should return a Readable | ||
| // We overwrite it so that we can get testing coverage for the scenario | ||
| // in onLine that would throw an error. | ||
| proc.stdout = null |
There was a problem hiding this comment.
According to the docs, it doesn't seem like I should have to do this but no matter what values I pass in the array to stdio it seems like proc.stdout is always set? I would love a second opinion here cc @vapurrmaid @bryphe-coder
There was a problem hiding this comment.
I think you might need different settings for cp.spawn to exercise this - checking
There was a problem hiding this comment.
If I use the stdio flag 'ignore', it seems to work as you expect (in order to exercise the code under test):
const proc = cp.spawn("node", [], {
stdio: 'ignore'
})
There was a problem hiding this comment.
At least, tested this out in the NodeJS console:
There was a problem hiding this comment.
Some useful docs around the various settings for options.stdio here: https://nodejs.org/api/child_process.html#optionsstdio
There was a problem hiding this comment.
Oh wow! Nice work 🙌 Okay I'll try that locally and see if it works for me as well.
There was a problem hiding this comment.
If I use the
stdioflag'ignore', it seems to work as you expect (in order to exercise the code under test):const proc = cp.spawn("node", [], { stdio: 'ignore' })
That worked!!!
There was a problem hiding this comment.
this one is better than my suggestion - glad it worked!
test/unit/node/util.test.ts
Outdated
| // which is why I have to set proc.stdout = null | ||
| // a couple lines below. | ||
| const proc = cp.spawn("node", [], { | ||
| stdio: [null], |
There was a problem hiding this comment.
Does
stdio: [0, "empty"]
work?
There was a problem hiding this comment.
Looks like TS doesn't like this but stdio: 'ignore' worked!
6c3105a to
afa5200
Compare
afa5200 to
d4ba9bb
Compare
This PR adds an additional test for the
onLineutil function to cover the scenario where the child.Process passed to the function is missing thestdoutmethod. This adds code coverage for this line.Screenshot
References
stdio- Node.js docsFixes N/A
#TestingMondays
Blocked by #4543