!Don't merge yet! Add unit tests #12
Conversation
sadym-chromium
force-pushed
the
unit
branch
2 times, most recently
from
acf3841 to
6dcf6c1
Compare
src/bidiMapper/utils/log.ts
Outdated
| @@ -15,16 +15,18 @@ | |||
| */ | |||
| export function log(type: string): (...message: any[]) => void { | |||
| return (...messages: any[]) => { | |||
| const elementId = type + '_log'; | |||
| console.log(type, ...messages); | |||
There was a problem hiding this comment.
This (or some other console.log) is showing up in the output of npm run unit, I see this:
CdpServer tests.
cdp sent > {"someAttribute":"someValue","id":0}
✔ given CdpServer, when `sendMessage` is called, then cdpBindings should be called with proper values
That "cdp sent" line would be nice to get rid of. If there is logging that's useful for debugging, then we should probably use a logging mechanism which can be disabled when running tests.
There was a problem hiding this comment.
done. Log only if run in browser.
src/bidiServerRunner.ts
Outdated
|
|
||
| const server = http.createServer(function (request, response) { | ||
| debugInternal(new Date() + ' Received request for ' + request.url); | ||
| response.writeHead(404); | ||
|
|
||
| // Needed for WPT compatibility. |
There was a problem hiding this comment.
This was already landed in https://github.com/GoogleChromeLabs/chromium-bidi/pull/7/files, why is it also showing up in this PR despite #7 being merged?
Hmm, I think it's because there are conflicts in package.json, so GitHub can't merge into main in order to compare against main...
|
On the mocking library to use, I've used Sinon with pure JS before. I don't exactly love it, but it gets the job done. Would it work to depend on Sinon and https://www.npmjs.com/package/@types/sinon perhaps? That has a lot of downloads at least, suggesting we'd not be alone :) |
|
@sadym-chromium , thanks for adding this. My next PR will bring some big changes to CdpClient, so it might be best to wait before creating a CdpClient unit test. Thanks! |
|
Closed in favour of #20, as |
UPD: Still not sure if the
ts-mockitois the best choice. It has a nice interface and some advantages comparing toSinon, like automatic creating mock by interfaces. But it looks like it has poor support: NagRock/ts-mockito#212