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

Server does not complete setup before next test suite executes in Mocha #637

Closed
cjones26 opened this issue Mar 8, 2021 · 16 comments
Closed
Labels
bug Something isn't working potentially solved scope:node Related to MSW running in Node

Comments

@cjones26
Copy link

cjones26 commented Mar 8, 2021

Environment

Name Version
msw 0.27.0
node 12.19.0
OS Linux x86_64

Request handlers

Note: I have created a repo displaying the behavior here.

App.test.js

import React from 'react';
import { render } from '@testing-library/react';
import { setupServer } from 'msw/node';
import { rest } from 'msw';
import App from '../App';

describe('App tests', () => {
  const server = setupServer(
    rest.post('http://localhost:8080/api/getList', (req, res, ctx) => {
      return res(
        ctx.set('access-control-allow-origin', '*'),
        ctx.json({
          msgCode: 0,
          msgDesc: 'Request processed successfully',
          data: {
            data: {
              createdBy: 'CJ',
              createdDate: '02/18/2021',
            },
            pagination: {
              pageNo: req.body.pagination.pageNo,
              pageSize: req.body.pagination.pageSize,
              total: 1,
              sortedColumn: req.body.pagination.sortedColumn,
              sortedType: req.body.pagination.sortedType,
            },
          },
        })
      );
    })
  );

  before(() => {
    server.listen();
  });

  after(() => {
    server.close();
  });

  it('renders App without crashing', () => {
    render(<App />);
  });
});

Child.test.js

import React from 'react';
import { render } from '@testing-library/react';
import { setupServer } from 'msw/node';
import { rest } from 'msw';
import Child from '../Child';

describe('Child tests', () => {
  const server = setupServer(
    rest.post('http://localhost:8080/api/getChildList', (req, res, ctx) => {
      return res(
        ctx.set('access-control-allow-origin', '*'),
        ctx.json({
          msgCode: 0,
          msgDesc: 'Request processed successfully',
          data: {
            data: {
              createdBy: 'CJ Child',
              createdDate: '02/18/2021',
            },
            pagination: {
              pageNo: req.body.pagination.pageNo,
              pageSize: req.body.pagination.pageSize,
              total: 1,
              sortedColumn: req.body.pagination.sortedColumn,
              sortedType: req.body.pagination.sortedType,
            },
          },
        })
      );
    })
  );

  before(() => {
    server.listen();
  });

  after(() => {
    server.close();
  });

  it('renders Child without crashing', () => {
    render(<Child />);
  });
});

Actual request

The actual request is simply performed on mount via the Fetch API (in our tests we are utilizing the isomorphic-fetch library to polyfill Fetch for Node).

function Child() {
  useEffect(() => {
    fetch('http://localhost:8080/api/getChildList', {
      method: 'POST',
      mode: 'cors',
      cache: 'no-cache',
      headers: {
        'Content-Type': 'application/json',
      },
      body: JSON.stringify({
        pagination: {
          pageSize: 10,
          pageNo: 0,
          sortedColumn: 'createdBy',
          sortedType: 'asc',
        },
      }),
    })
      .then(async (response) => {
        console.log('Fetch complete: ', await response.json());
      })
      .catch((error) => {
        console.error('Failed to fetch: ', error);
      });
  }, []);

  return (
    <React.Fragment>
      <h1>Child</h1>
    </React.Fragment>
  );
}

Current behavior

If we are running two test suites consecutively, in this case, App.test.js and then Child.test.js in quick succession, the second server is not fully set up before the test executes, resulting in something like this:

> mocha-ref@1.0.0 test:watch
> cross-env NODE_ENV=test mocha --timeout 10000 -w src/**/*.test.js



  App tests
    ✓ renders App without crashing
Fetch complete:  {
  msgCode: 0,
  msgDesc: 'Request processed successfully',
  data: {
    data: { createdBy: 'CJ', createdDate: '02/18/2021' },
    pagination: {
      pageNo: 0,
      pageSize: 10,
      total: 1,
      sortedColumn: 'createdBy',
      sortedType: 'asc'
    }
  }
}

  Child tests
    ✓ renders Child without crashing
Failed to fetch:  FetchError: request to http://localhost:8080/api/getChildList failed, reason: connect ECONNREFUSED 127.0.0.1:8080
    at ClientRequest.<anonymous> (/home/cjones26/JS/mocha-ref/node_modules/node-fetch/lib/index.js:1461:11)
    at ClientRequest.emit (events.js:314:20)
    at Socket.socketErrorListener (_http_client.js:428:9)
    at Socket.emit (events.js:314:20)
    at emitErrorNT (internal/streams/destroy.js:92:8)
    at emitErrorAndCloseNT (internal/streams/destroy.js:60:3)
    at processTicksAndRejections (internal/process/task_queues.js:84:21) {
  type: 'system',
  errno: 'ECONNREFUSED',
  code: 'ECONNREFUSED'
}


  2 passing (59ms)

ℹ [mocha] waiting for changes...

Expected behavior

> mocha-ref@1.0.0 test:watch
> cross-env NODE_ENV=test mocha --timeout 10000 -w src/**/*.test.js



  App tests
    ✓ renders App without crashing
Fetch complete:  {
  msgCode: 0,
  msgDesc: 'Request processed successfully',
  data: {
    data: { createdBy: 'CJ', createdDate: '02/18/2021' },
    pagination: {
      pageNo: 0,
      pageSize: 10,
      total: 1,
      sortedColumn: 'createdBy',
      sortedType: 'asc'
    }
  }
}

  Child tests
    ✓ renders Child without crashing
Fetch complete:  {
  msgCode: 0,
  msgDesc: 'Request processed successfully',
  data: {
    data: { createdBy: 'CJ Child', createdDate: '02/18/2021' },
    pagination: {
      pageNo: 0,
      pageSize: 10,
      total: 1,
      sortedColumn: 'createdBy',
      sortedType: 'asc'
    }
  }
}

  2 passing (59ms)

ℹ [mocha] waiting for changes...

Screenshots

Failed test:
Failed test

@cjones26 cjones26 added bug Something isn't working scope:node Related to MSW running in Node labels Mar 8, 2021
@kettanaito
Copy link
Member

Hey, @cjones26. Thanks for raising this.

I tend to believe this has the same root cause as #474, which comes down to the node-request-interceptor library patching request modules per entire process.

Can you please try running the same tests but in sequence? Do you still see the error then?

@msutkowski
Copy link
Member

@kettanaito You might be right, but I'm not 100% sure and it's kind of a pain to debug. By default, mocha runs sequentially and you have to opt-in to parallel=true. This seems to be more of a problem with how mocha is running tests, but I'm no mocha expert.

For example, in the repo as-is, if you remove the server.close() from each test file, it'll run them all just fine but would lead to cleanup issues.

As an immediate solution that works with both sequential and parallel runs, what I'd recommend @cjones26 does is change the mocha configuration with these steps:

  1. Use root hooks - https://mochajs.org/#defining-a-root-hook-plugin (see below)
  2. Update mocha config (see below)
  3. Add hooks for the msw server (see below)
  4. Cleanup tests (see below)

In the given repro, you'd add this file:

// test/hooks.js
import { setupServer } from "msw/node";
import { rest } from "msw";

// export the server instance so you can `server.use` as needed
export const server = setupServer(
  rest.post("http://localhost:8080/api/getList", (req, res, ctx) => {
    return res(
      ctx.set("access-control-allow-origin", "*"),
      ctx.json({
        msgCode: 0,
        msgDesc: "Request processed successfully",
        data: {
          data: {
            createdBy: "CJ",
            createdDate: "02/18/2021",
          },
          pagination: {
            pageNo: req.body.pagination.pageNo,
            pageSize: req.body.pagination.pageSize,
            total: 1,
            sortedColumn: req.body.pagination.sortedColumn,
            sortedType: req.body.pagination.sortedType,
          },
        },
      })
    );
  }),
  rest.post("http://localhost:8080/api/getChildList", (req, res, ctx) => {
    return res(
      ctx.set("access-control-allow-origin", "*"),
      ctx.json({
        msgCode: 0,
        msgDesc: "Request processed successfully",
        data: {
          data: {
            createdBy: "CJ Child",
            createdDate: "02/18/2021",
          },
          pagination: {
            pageNo: req.body.pagination.pageNo,
            pageSize: req.body.pagination.pageSize,
            total: 1,
            sortedColumn: req.body.pagination.sortedColumn,
            sortedType: req.body.pagination.sortedType,
          },
        },
      })
    );
  })
);

exports.mochaHooks = {
  beforeAll: function () {
    server.listen();
  },
  afterEach: function () {
    server.resetHandlers();
  },
  afterAll: function () {
    server.close();
  },
};

Update mocha config to include the new hooks file:

// .mocharc.yaml
require:
  - "test/setupMocha.js"
  - "test/specHelper.js"
  - "test/hooks.js"

Refactor tests to look like:

import React from "react";
import { render } from "@testing-library/react";
import Child from "../Child";

describe("Child tests", () => {
  it("renders Child without crashing", () => {
    render(<Child />);
  });
});

and

import React from "react";
import { render } from "@testing-library/react";
import App from "../App";

describe("App tests", () => {
  it("renders App without crashing", async () => {
    render(<App />);
  });
});

As general guidance, I'd move the request handlers and server instance creation out into their own files so you can more easily reuse them in other places such as Storybook.

@cjones26
Copy link
Author

cjones26 commented Mar 9, 2021

@msutkowski -- you beat me to responding but yes it does appear to be something with the way that Mocha is working internally, and agree that it would be a pain to debug. I have not looked into the implementation of the functions, but my thought was that it could be helpful if server.listen and server.close both returned a promise which resolved once the server was fully started and/or fully cleaned up.

As for your suggestion above, I believe this will work for our use case and I will refactor our real project (not the provided sample) in order to validate this.

Thanks!

@msutkowski
Copy link
Member

@msutkowski -- you beat me to responding but yes it does appear to be something with the way that Mocha is working internally, and agree that it would be a pain to debug. I have not looked into the implementation of the functions, but my thought was that it could be helpful if server.listen and server.close both returned a promise which resolved once the server was fully started and/or fully cleaned up.

As for your suggestion above, I believe this will work for our use case and I will refactor our real project (not the provided sample) in order to validate this.

Thanks!

Awesome! Let us know how it goes. I'm assuming it'll work as expected, and if so, I'll go ahead and open a PR for a mocha example and/or recipe for future users.

@cjones26
Copy link
Author

cjones26 commented Mar 10, 2021

@msutkowski -- the changes work....but only when we are not in watch mode. Once we run in watch mode and attempt to make any changes, the entire thing comes crashing down as shown here:

Failed after save

Do you have any other suggestions, or perhaps any thoughts on my suggestion earlier, regarding returning a promise from the server.listen and server.close methods? Or maybe a "done" callback which we can pass in while creating the server which will be called after setup / teardown are complete?

Thanks again!

@msutkowski
Copy link
Member

@msutkowski -- the changes work....but only when we are not in watch mode. Once we run in watch mode and attempt to make any changes, the entire thing comes crashing down as shown here:

Do you have any other suggestions, or perhaps any thoughts on my suggestion earlier, regarding returning a promise from the server.listen and server.close methods? Or maybe a "done" callback which we can pass in while creating the server which will be called after setup / teardown are complete?

Okay, I think I have the right solution this time: https://mochajs.org/#global-fixtures, which as documented, is exactly how msw should be used.

Instead of hooks.js, rename to fixtures.js and update your mocharc to include the below

import { server } from "./setupServer"; // `createServer` code moved here as mentioned in previous comment

export function mochaGlobalSetup() {
  server.listen();
  server.printHandlers(); // optional, but helpful!
  console.log(`msw server started!`);
}

export function mochaGlobalTeardown() {
  server.close();
  console.log("server stopped!");
}

Then in your individual tests, just call server.resetHandlers() in before.

References: mochajs/mocha#4347 - looks like this lead to the concept of global fixtures released here: https://github.com/mochajs/mocha/releases/tag/v8.2.0

@cjones26
Copy link
Author

@msutkowski -- ahhh, it's extremely close to working. Unfortunately, when we are using watch mode, if a handler is overridden, on save the server.resetHandlers doesn't seem to want to restore the original handler (I've tried it in before, beforeEach, after, and afterEach--none work). Take a look below and you'll see what I mean--once I override the handler (and hit save), it never actually resets to what it needs to be ("Initial handler" vs "Overridden handler" in the data.data.createdBy field):

> mocha-ref@1.0.0 test:watch
> cross-env NODE_ENV=test mocha --timeout 10000 -w src/**/*.test.js

[rest] POST https://localhost:8080/api/getList
  Declaration: /home/corte/JS/mocha-ref/test/server.js:7:8

[rest] POST https://localhost:8080/api/getChildList
  Declaration: /home/corte/JS/mocha-ref/test/server.js:29:8

msw server started!


  App tests
    - renders App without crashing

  Child tests
    ✓ renders Child again but we expect createdBy to be set back to N/A
Fetch complete:  {
  msgCode: 0,
  msgDesc: 'Request processed successfully',
  data: {
    data: { createdBy: 'Original Handler', createdDate: '02/18/2021' },
    pagination: {
      pageNo: 0,
      pageSize: 10,
      total: 1,
      sortedColumn: 'createdBy',
      sortedType: 'asc'
    }
  }
}
    ✓ renders Child without crashing
Fetch complete:  {
  msgCode: 0,
  msgDesc: 'Request processed successfully',
  data: {
    data: { createdBy: 'Overridden handler', createdDate: '03/10/2021' },
    pagination: {
      pageNo: 0,
      pageSize: 10,
      total: 1,
      sortedColumn: 'createdBy',
      sortedType: 'asc'
    }
  }
}


  2 passing (61ms)
  1 pending

ℹ [mocha] waiting for changes...


  App tests
    - renders App without crashing

  Child tests
    ✓ renders Child again but we expect createdBy to be set back to N/A
Fetch complete:  {
  msgCode: 0,
  msgDesc: 'Request processed successfully',
  data: {
    data: { createdBy: 'Overridden handler', createdDate: '03/10/2021' },
    pagination: {
      pageNo: 0,
      pageSize: 10,
      total: 1,
      sortedColumn: 'createdBy',
      sortedType: 'asc'
    }
  }
}
    ✓ renders Child without crashing
Fetch complete:  {
  msgCode: 0,
  msgDesc: 'Request processed successfully',
  data: {
    data: { createdBy: 'Overridden handler', createdDate: '03/10/2021' },
    pagination: {
      pageNo: 0,
      pageSize: 10,
      total: 1,
      sortedColumn: 'createdBy',
      sortedType: 'asc'
    }
  }
}


  2 passing (27ms)
  1 pending

ℹ [mocha] waiting for changes...

@cjones26
Copy link
Author

cjones26 commented Mar 12, 2021

An update here--something funky is going on with node-request-interceptor (as suggested initially by @kettanaito). After saving repeatedly in watch mode, I begin receiving the following warning:

[WARNING]:  MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 end listeners added to [IncomingMessage]. Use emitter.setMaxListeners() to increase limit
    at _addListener (events.js:389:17)
    at IncomingMessage.addListener (events.js:405:10)
    at IncomingMessage.Readable.on (_stream_readable.js:874:35)
    at IncomingMessage.once (events.js:436:8)
    at ClientRequestOverride.<anonymous> (/home/corte/JS/mocha-ref/node_modules/node-fetch/lib/index.js:1546:8)
    at ClientRequestOverride.emit (events.js:314:20)
    at ClientRequestOverride.<anonymous> (../../../src/interceptors/ClientRequest/ClientRequestOverride.ts:310:14)
    at ClientRequestOverride.emit (events.js:326:22)
    at ClientRequestOverride.<anonymous> (../../../src/interceptors/ClientRequest/ClientRequestOverride.ts:310:14)
    at ClientRequestOverride.emit (events.js:326:22)
    at ClientRequestOverride.<anonymous> (../../../src/interceptors/ClientRequest/ClientRequestOverride.ts:310:14)
    at ClientRequestOverride.emit (events.js:326:22)
    at ClientRequestOverride.<anonymous> (../../../src/interceptors/ClientRequest/ClientRequestOverride.ts:310:14)
    at ClientRequestOverride.emit (events.js:326:22)
    at ClientRequestOverride.<anonymous> (../../../src/interceptors/ClientRequest/ClientRequestOverride.ts:310:14)
    at ClientRequestOverride.emit (events.js:326:22)
    at ClientRequestOverride.<anonymous> (../../../src/interceptors/ClientRequest/ClientRequestOverride.ts:310:14)
    at ClientRequestOverride.emit (events.js:326:22)
    at ClientRequestOverride.<anonymous> (../../../src/interceptors/ClientRequest/ClientRequestOverride.ts:310:14)
    at ClientRequestOverride.emit (events.js:326:22)
    at ClientRequestOverride.<anonymous> (../../../src/interceptors/ClientRequest/ClientRequestOverride.ts:310:14)
    at ClientRequestOverride.emit (events.js:326:22)
    at ClientRequestOverride.<anonymous> (../../../src/interceptors/ClientRequest/ClientRequestOverride.ts:310:14)
    at ClientRequestOverride.emit (events.js:326:22)
    at ClientRequestOverride.<anonymous> (../../../src/interceptors/ClientRequest/ClientRequestOverride.ts:310:14)
    at ClientRequestOverride.emit (events.js:326:22)
    at ClientRequestOverride.<anonymous> (../../../src/interceptors/ClientRequest/ClientRequestOverride.ts:234:14)
    at step (/home/corte/JS/mocha-ref/node_modules/node-request-interceptor/lib/interceptors/ClientRequest/ClientRequestOverride.js:128:17)
    at Object.next (/home/corte/JS/mocha-ref/node_modules/node-request-interceptor/lib/interceptors/ClientRequest/ClientRequestOverride.js:59:14)
    at fulfilled (/home/corte/JS/mocha-ref/node_modules/node-request-interceptor/lib/interceptors/ClientRequest/ClientRequestOverride.js:13:24)

Clearly setupServer is being called multiple times--I will look further into this in the next few hours to see what I can find out.

@msutkowski
Copy link
Member

An update here--something funky is going on with node-request-interceptor (as suggested initially by @kettanaito). After saving repeatedly in watch mode, I begin receiving the following warning:

[WARNING]:  MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 end listeners added to [IncomingMessage]. Use emitter.setMaxListeners() to increase limit
    at _addListener (events.js:389:17)
    at IncomingMessage.addListener (events.js:405:10)
    at IncomingMessage.Readable.on (_stream_readable.js:874:35)
    at IncomingMessage.once (events.js:436:8)
    at ClientRequestOverride.<anonymous> (/home/corte/JS/mocha-ref/node_modules/node-fetch/lib/index.js:1546:8)
    at ClientRequestOverride.emit (events.js:314:20)
    at ClientRequestOverride.<anonymous> (../../../src/interceptors/ClientRequest/ClientRequestOverride.ts:310:14)
    at ClientRequestOverride.emit (events.js:326:22)
    at ClientRequestOverride.<anonymous> (../../../src/interceptors/ClientRequest/ClientRequestOverride.ts:310:14)
    at ClientRequestOverride.emit (events.js:326:22)
    at ClientRequestOverride.<anonymous> (../../../src/interceptors/ClientRequest/ClientRequestOverride.ts:310:14)
    at ClientRequestOverride.emit (events.js:326:22)
    at ClientRequestOverride.<anonymous> (../../../src/interceptors/ClientRequest/ClientRequestOverride.ts:310:14)
    at ClientRequestOverride.emit (events.js:326:22)
    at ClientRequestOverride.<anonymous> (../../../src/interceptors/ClientRequest/ClientRequestOverride.ts:310:14)
    at ClientRequestOverride.emit (events.js:326:22)
    at ClientRequestOverride.<anonymous> (../../../src/interceptors/ClientRequest/ClientRequestOverride.ts:310:14)
    at ClientRequestOverride.emit (events.js:326:22)
    at ClientRequestOverride.<anonymous> (../../../src/interceptors/ClientRequest/ClientRequestOverride.ts:310:14)
    at ClientRequestOverride.emit (events.js:326:22)
    at ClientRequestOverride.<anonymous> (../../../src/interceptors/ClientRequest/ClientRequestOverride.ts:310:14)
    at ClientRequestOverride.emit (events.js:326:22)
    at ClientRequestOverride.<anonymous> (../../../src/interceptors/ClientRequest/ClientRequestOverride.ts:310:14)
    at ClientRequestOverride.emit (events.js:326:22)
    at ClientRequestOverride.<anonymous> (../../../src/interceptors/ClientRequest/ClientRequestOverride.ts:310:14)
    at ClientRequestOverride.emit (events.js:326:22)
    at ClientRequestOverride.<anonymous> (../../../src/interceptors/ClientRequest/ClientRequestOverride.ts:234:14)
    at step (/home/corte/JS/mocha-ref/node_modules/node-request-interceptor/lib/interceptors/ClientRequest/ClientRequestOverride.js:128:17)
    at Object.next (/home/corte/JS/mocha-ref/node_modules/node-request-interceptor/lib/interceptors/ClientRequest/ClientRequestOverride.js:59:14)
    at fulfilled (/home/corte/JS/mocha-ref/node_modules/node-request-interceptor/lib/interceptors/ClientRequest/ClientRequestOverride.js:13:24)

Clearly setupServer is being called multiple times--I will look further into this in the next few hours to see what I can find out.

@cjones26 Sorry about the delay - I was doing some debugging on this yesterday, but had to stop. I'm going to push up my forked repo this afternoon. It's possible that NRI is part of the problem, but there is also just an issue with the test setup and how mocha is running things. For example, even if you make tests async, render and use a util like await waitFor, the tests just all execute immediately. I'd prefer to eliminate all of the mocha noise before we pin it on NRI specifically :)

@cjones26
Copy link
Author

cjones26 commented Mar 12, 2021

@msutkowski no worries at all! And completely understood--I would agree that we need to eliminate Mocha setup as the cause before pointing fingers at NRI. I will continue playing around to see whether I can resolve the issue as well.

@msutkowski
Copy link
Member

@cjones26 PR opened here: cjones26/mocha-ref#2 with notes.

That works, but it still seems strange to me. I'm hoping @kettanaito, @marcosvega91 or @timdeschryver might have an idea of a better implementation.

@marcosvega91
Copy link
Member

@msutkowski I have looked into the code and update it a little bit using the before and after function in a global scope. I have add my code here. Let me know what you think

@msutkowski
Copy link
Member

@marcosvega91 Thanks again! I merged that into my repo and into the PR for @cjones26. I think this is as good as we're going to get for right now. We will have to investigate supporting parallel mode, which may or may not have to do with #474 as @kettanaito said :)

@cjones26
Copy link
Author

cjones26 commented Mar 15, 2021

@msutkowski @marcosvega91 -- thanks for all the help on this one. What was the exact change which allowed this to start working properly? Was it moving around the setup in .mocharc, or was it using Common JS import style instead of ES6 style so that the server instance was cached properly by Mocha?

Also, on a side note these changes do work properly in our production project as well 🍾 🥳 !

@msutkowski
Copy link
Member

@cjones26 The magic here is the usage of the --file flag in the CLI (or in mocharc in this case). The server creation/cleanups are always rerun and skip the awkward handling of the lifecycle hooks behavior from mocha.

This behavior is slightly different than what I did originally where I created the server once, then set it to global.

Either works, but this is the best we can do for now. We're going to continue to research supporting parallel mode - if we resolve that, I'll let you know so you can update if necessary. Thanks again for providing this issue!

@cjones26
Copy link
Author

@msutkowski -- sure thing -- as of now I am sure we are fine and won't need parallel mode. Thanks again, closing this out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working potentially solved scope:node Related to MSW running in Node
Projects
None yet
Development

No branches or pull requests

4 participants