Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jdp debugger client tests #6

Open
wants to merge 3 commits into
base: jerry-debugger-proxy
Choose a base branch
from

Conversation

grgustaf
Copy link
Collaborator

@grgustaf grgustaf commented Mar 2, 2018

No description provided.

@grgustaf
Copy link
Collaborator Author

grgustaf commented Mar 2, 2018

@martijnthe, when I just said jest.mock('ws') that worked up until this last test I'm trying to write, but I'm wanting to emit events on the websocket to get the on('open') handler called and such. Here is my not working attempt so far...

@martijnthe martijnthe force-pushed the jerry-debugger-proxy branch from 04eefab to da112ed Compare March 12, 2018 20:18
@martijnthe
Copy link
Owner

Ah sorry I missed this PR and it's conflicting with the changes I made w.r.t. switching to the nicer CLI arg parser. Mind rebasing / updating this?

@grgustaf
Copy link
Collaborator Author

Yeah, I'll try to get back to this at some point.

@grgustaf grgustaf added the WIP label Mar 21, 2018
@grgustaf grgustaf force-pushed the jerry-debugger-proxy branch 2 times, most recently from 70135ee to b584d9d Compare April 6, 2018 17:29
JerryScript-DCO-1.0-Signed-off-by: Geoff Gustafson [email protected]
@grgustaf grgustaf force-pushed the jdp-debugger-client-tests branch from 67231f0 to e85a555 Compare April 10, 2018 22:33
@grgustaf
Copy link
Collaborator Author

I tried and failed to mock stuff like on() and emit() in the WebSocket module to test additional the 'open' and 'error' cases etc, but just couldn't make it work. Part of the difficulty I think is that the WebSocket module is a constructor. Maybe later I'll have learned enough about Jest to make it work, but moving on to other things.

@grgustaf
Copy link
Collaborator Author

Realized lint was failing - fixed two issues but the third one requires me to return the promise - if I do I realize it's not getting resolved, and I'm back to that problem where I can't figure out how to override functions in the WebSocket mock to provide this functionality. @martijnthe, any insight?

@martijnthe
Copy link
Owner

@grgustaf this fixes it:

diff --git a/jerry-debugger/jerry-client-ts/src/lib/__tests__/debugger-client.test.ts b/jerry-debugger/jerry-client-ts/src/lib/__tests__/debugger-client.test.ts
index 278818dd..82cebeea 100644
--- a/jerry-debugger/jerry-client-ts/src/lib/__tests__/debugger-client.test.ts
+++ b/jerry-debugger/jerry-client-ts/src/lib/__tests__/debugger-client.test.ts
@@ -42,7 +42,7 @@ describe('JerryDebugger.connect', () => {
   it('creates a websocket', () => {
     const delegate = {} as any;
     const jd = new JerryDebuggerClient({ delegate });
-    jd.connect();
+    jd.connect().catch((e) => { throw e; });
     expect(WebSocket).toHaveBeenCalledTimes(1);
   });
 });

You could also use await jd.connect(); (and make the test function async) but then you'll have to change the mock to also resolve, otherwise the test will time out.
If not using await you need to at least .catch() the result of promise, because otherwise if there was an error being thrown inside the Promise executor function, it could go by unnoticed.

@martijnthe
Copy link
Owner

Maybe later I'll have learned enough about Jest to make it work, but moving on to other things.

OK, sounds good.

JerryScript-DCO-1.0-Signed-off-by: Geoff Gustafson [email protected]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants