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

[NEW TEST] WebSockets #14

Closed
jmcdo29 opened this issue Nov 15, 2019 · 4 comments · Fixed by #601
Closed

[NEW TEST] WebSockets #14

jmcdo29 opened this issue Nov 15, 2019 · 4 comments · Fixed by #601
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@jmcdo29
Copy link
Owner

jmcdo29 commented Nov 15, 2019

Feature Test To Be Requested

A test repository showing how to test web sockets could be really useful for some people. Full End to End testing too if possible

@jmcdo29 jmcdo29 added the enhancement New feature or request label Nov 15, 2019
@jmcdo29 jmcdo29 self-assigned this Nov 15, 2019
@jmcdo29 jmcdo29 added this to To do in Test Scenarios via automation Nov 15, 2019
@jmcdo29 jmcdo29 removed their assignment Nov 15, 2019
@jmcdo29 jmcdo29 added good first issue Good for newcomers help wanted Extra attention is needed labels Nov 15, 2019
jmcdo29 added a commit that referenced this issue Nov 26, 2019
A very simple websocket server (single enpoint). Client and server code written and ready to play
with

fix #14
jmcdo29 added a commit that referenced this issue Nov 26, 2019
A very simple websocket server (single enpoint). Client and server code written and ready to play
with

fix #14
Test Scenarios automation moved this from To do to Done Nov 26, 2019
@ShGKme
Copy link

ShGKme commented Mar 8, 2020

Tere is an issue with e2e testing.
Jest did not exit one second after the test run has completed.

To be honest, I can't understand, why this:
https://github.com/jmcdo29/testing-nestjs/blob/master/apps/websocket-sample/test/app.e2e-spec.ts#L23
will connect exacly to the testing app http server... How it works.

And what about adding about more "websocket" test. In this test event just return result like usual http endpoint. It'd be more interesting to return WsResponse (emit another event as an answer to subscribed event).

@jmcdo29
Copy link
Owner Author

jmcdo29 commented Mar 9, 2020

@ShGKme I'll be 100% honest, I'm not completely sure how it works myself. I believe it has something to do with socket.io scanning already made connections via socket.io, and creating a client for it. If you have an idea for better tests I'd be happy to accept a PR for them. Websockets are still a bit out of my realm of knowledge so I went with a simple test for now. I'll re-open the issue for now as a reminder that better tests are needed. Maybe even one for non-socket.io clients

@jmcdo29 jmcdo29 reopened this Mar 9, 2020
Test Scenarios automation moved this from Done to In progress Mar 9, 2020
@ShGKme
Copy link

ShGKme commented Mar 9, 2020

@jmcdo29 io.connect(); doesn't really work.
It's very funny. Let's look to the test:

// It's an async test, so, it except Promise to be resolved.
it('should call message', async () => {
  // Connect to socket server (what server? does it know an address?)
  const socket = io.connect();
  // Does we connect successfully? connect never throw error itself...

  //Ok, let's emit a message
  socket.emit('message', { name: 'Test' }, (data) => {
    // (!)When(!) it'll receive an acknowledgement, we will check, that it's correct
    expect(data).toBe('Hello, Test!');
  });
  // (!) If we won't receive any acknowledgement from message event, we will check nothing :D

  // By the way, disconnect doesn't return Promise.
  return socket.disconnect();
});

The test will finish earler, than we will receive anything and won't check for any errors.
At the end, we have passed test with a Jest warning:

Jest did not exit one second after the test run has completed.
This usually means that there are asynchronous operations that weren't stopped in your tests.

More fun? This will pass :D

it('should call message', async () => {
  const socket = io.connect('http://THE_URL_THAT_DOES_NOT_EXISTS:67000');
  socket.emit('NO-EVENT', { name: 'Test' }, (data) => {
    expect(data).toBe('LITERALLY ANY VALUE');
    throw new Error('JUST GO THROW AN ERROR HERE');
  });
  return socket.disconnect();
});

I will make my example soon today.

@ShGKme
Copy link

ShGKme commented Mar 9, 2020

@jmcdo29 Check out this repository:
https://github.com/ShGKme/nestjs-ws-e2e

Or this repl (run it in browser):
https://repl.it/github/ShGKme/nestjs-ws-e2e

jmcdo29 added a commit that referenced this issue May 24, 2020
Because I was using `io.connect()` without a properport, socketIo was not properly disconnecting
after the test. Now that I am useing `io.connect(await app.getUrl())` everything works just fine

fix #14
jmcdo29 added a commit that referenced this issue May 25, 2020
Because I was using `io.connect()` without a properport, socketIo was not properly disconnecting
after the test. Now that I am useing `io.connect(await app.getUrl())` everything works just fine

fix #14
@jmcdo29 jmcdo29 linked a pull request May 25, 2020 that will close this issue
Test Scenarios automation moved this from In progress to Done May 25, 2020
jmcdo29 added a commit that referenced this issue May 25, 2020
* chore: moves to eslint and starts dockerization

* fix(websocket): fix jest not exiting properly

Because I was using `io.connect()` without a properport, socketIo was not properly disconnecting
after the test. Now that I am useing `io.connect(await app.getUrl())` everything works just fine

fix #14

* ci: starts dockerizing the e2e testing process

I'm tired of seeing CRON failures so I'm also adding in the necessary service to the cron.yml for
the moment. I'll get it all fixed in the future

* test(typeorm): fixes up typeorm e2e tests with docker-compose

* feat: removes typeorm-graphql-sample

The TypeOrm-GraphQL-Sample project is removed as there is no need for two sample projects to use
TypeORM. The point of the repository is to show how testing can be done with NestJS. As there is a
fully working typeorm-sample project and a graphql-sample project, there is no need to combine the
two.

BREAKING CHANGE: removal of the typeorm-graphql-sample

* feat(e2e): adds docker-compose for e2e tests

There's still some bugs with how the microservices tests were implemented, I'll need to spend more
time on that, but now the TypeORM test has an actual database to query and use. Mongo is next, but I
want to get rid of the failing CRON test for now.

* fix: fixes microservice tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
Development

Successfully merging a pull request may close this issue.

2 participants