test: adding mustCall in test-fs-readfile-empty.js #27455
Conversation
ZYSzys
added
the
code-and-learn
|
|
||
| const fn = fixtures.path('empty.txt'); | ||
|
|
||
| fs.readFile(fn, function(err, data) { | ||
| fs.readFile(fn, mustCall(function (err, data) { |
There was a problem hiding this comment.
please remove the space after function:
function (err, data) -> function(err, data)
| @@ -27,20 +27,21 @@ require('../common'); | |||
| const assert = require('assert'); | |||
| const fs = require('fs'); | |||
| const fixtures = require('../common/fixtures'); | |||
| const { mustCall } = require('../common') | |||
There was a problem hiding this comment.
Hi, missing semicolon here:
| const { mustCall } = require('../common') | |
| const { mustCall } = require('../common'); |
Besides, remember to remove the duplicated require('..common'); in line 23 above if you require('../common') here.
|
Nit/note for whoever lands this: Please change |
| @@ -20,27 +20,27 @@ | |||
| // USE OR OTHER DEALINGS IN THE SOFTWARE. | |||
|
|
|||
| 'use strict'; | |||
| require('../common'); | |||
There was a problem hiding this comment.
The common module should be required in tests before any other module: https://github.com/nodejs/node/blob/master/doc/guides/writing-tests.md#lines-1-3
I’m a little surprised that the linter didn’t pick this up.
|
Landed in 9c2774e, thanks! You can visit https://www.nodetodo.org/next-steps/ for more ideas to contribute. |
PR-URL: #27455 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ouyang Yadong <[email protected]>
PR-URL: #27455 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ouyang Yadong <[email protected]>
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes