test: migrate common.js tests to tap #176
Conversation
test/node/common-safe-require.js
Outdated
| tap.ok(common.safeRequire(existingModule), 'must require existing module'); | ||
|
|
||
| tap.equal(common.safeRequire(nonExistingModule), null, | ||
| 'must return \'null\' if module doesn\'t exist'); |
There was a problem hiding this comment.
It will be better to avoid escaping by using backticks or double quotes here, but this is not mandatory.
There was a problem hiding this comment.
@belochub currently eslint forbids to do so.
|
@metarhia/jstp-core I think we may merge |
test/node/common-forward-event.js
Outdated
| targetEventEmitter.on('renamedEvent', spy); | ||
|
|
||
| sourceEventEmitter.emit('testEvent'); | ||
| test.assert(spy.called, 'event handler must be called'); |
There was a problem hiding this comment.
Ain't it 'must have been called'?
There was a problem hiding this comment.
@lundibundi it should have been if it was a sentence, but in this case it's just a test name. Look at the other assertions, they are written in the same style.
test/node/common-forward-event.js
Outdated
| const targetEventEmitter = new events.EventEmitter(); | ||
|
|
||
| common.forwardEvent(sourceEventEmitter, targetEventEmitter, | ||
| 'testEvent', 'renamedEvent'); |
test/node/common-safe-require.js
Outdated
|
|
||
| const common = require('../../lib/common'); | ||
|
|
||
| const existingModule = 'tap'; |
There was a problem hiding this comment.
Maybe it would be better to require a core module here, but this works for me too.
test/node/common-safe-require.js
Outdated
| const common = require('../../lib/common'); | ||
|
|
||
| const existingModule = 'tap'; | ||
| const nonExistingModule = 'tp'; |
There was a problem hiding this comment.
And it might be better to use something more explicitly non-existent like __not_a_module here.
test/node/common-safe-require.js
Outdated
| const existingModule = 'tap'; | ||
| const nonExistingModule = 'tp'; | ||
| const existingModule = '../..'; | ||
| const nonExistingModule = '__non_existinting_module__'; |
|
@nechaido CI's failing: |
|
@aqrln yea, I see it. Unfortunately I had no time to fix it until now. |
|
@aqrln fixed. |
|
Landed in a842a5f. |
PR-URL: metarhia/jstp#176 Refs: metarhia/jstp#145 Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com> Reviewed-By: Mykola Bilochub <nbelochub@gmail.com>
PR-URL: metarhia/jstp#176 Refs: metarhia/jstp#145 Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com> Reviewed-By: Mykola Bilochub <nbelochub@gmail.com>
Also add sinon to dependencies.