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

testdouble.replace + require kills wallaby instance #550

Closed
smeijer opened this Issue Apr 9, 2016 · 3 comments

Comments

Projects
None yet
2 participants
@smeijer
Copy link

smeijer commented Apr 9, 2016

Somehow rewiring requires with testdouble is killing Wallaby. I don't know how to explain it otherwise, but if any questions; please ask. I have set up an repository with minimal replicated scenario. You can see that the same case by using proxyquire works.

Repeatedly changing /home in tests/post-testdouble.js#L35 to anything different (try remove the e) crashes Wallaby. Repeatedly because the first edit seems to go just fine. It's the second and subsequent call that kills it.

Doing this very same thing in tests/post-proxyquire-sinon.js#L35 or tests/post-proxyquire-testdouble.js#L35 has no effect. Wallaby keeps running just fine.

To verify it is an wallaby issue, and not one of testdouble, I have setup a plain mocha test. Run it with npm run test to see nice green checkmarks. The script is setup to keep watching, so while toggling the values as written above, you see that mocha reports errors when appropriate and green check marks otherwise.

c:\dev\wallaby-testcase (master)
$ npm run test

  proxyquire with sinon
    √ now stubs Meteor packages
    √ can spy on Meteor packages

  proxyquire with testdouble
    √ now stubs Meteor packages
    √ can spy on Meteor packages

  testdouble
    √ now stubs Meteor packages
    √ can spy on Meteor packages


  6 passing (19ms)

Some technical data:

OS: Windows 10 Home
IDE: WebStorm 2016.1
Wallaby.js: https://github.com/smeijer/wallabyjs-issue-550/blob/master/wallaby.js
Repo: https://github.com/smeijer/wallabyjs-issue-550

@ArtemGovorov

This comment has been minimized.

Copy link
Member

ArtemGovorov commented Apr 9, 2016

It may work in plain mocha and not in wallaby (especially considering that it works before the first edit), because wallaby by default re-uses node processes.

Some libraries (and maybe testdouble is the case) are keeping some state inside (or even leaving some global state), so may fail after a single test run. The simplest solution is to set the workers: {recycle: true} setting. Please let me know if the solution works for you.

An alternative solution would be to find out what exactly causes the error and try to clean things up in a setup or teardown function.

ArtemGovorov added a commit to ArtemGovorov/wallabyjs-issue-550 that referenced this issue Apr 9, 2016

@smeijer

This comment has been minimized.

Copy link

smeijer commented Apr 9, 2016

Okay; I'm feeling very stupid at the moment. This is a great example for pointing out that it's important to cleanup your mess, euhm, tests.

Your suggestion of setting workers: { recycle: true } works indeed, but that's a workaround. The real problem is that I did not cleanup my garbage.

I've fixed it by adding an after() to the tests, as seen here:

import td from 'testdouble';
td.replace('...', {}); // because of this

describe('my test', () => {
  after(() => {
    td.reset(); // we must writ this
  });
});

So thank you @ArtemGovorov for your kind and fast assistance, and again; I'm sorry to be of trouble. Please see this as an signal that I'm very happy with the trial you guys provided me, and that I really want to keep working with your awesome product.

@smeijer smeijer closed this Apr 9, 2016

@ArtemGovorov

This comment has been minimized.

Copy link
Member

ArtemGovorov commented Apr 9, 2016

Thanks for the update. No worries at all, happy to help.

Please note that doing the td.reset in the after may not be a bulletproof solution, wallaby can't guarantee the after function will always be invoked (even plain mocha can't always guarantee it).

Being a continuous test runner, wallaby sometimes needs to cancel your current test run in a middle and it's not always possible to do it gracefully. For example because of an introduced infinite loop (as a result of partially declared loop as you type) or some long running async tests that has to be cancelled because they are still running while there's already a new code change has arrived, etc.

So it may be a good idea to add the same to your setup or teardown function, for example this should clean the testdouble state (it hopefully should not matter whether it was already created for the node process or not):

setup: () => {
  require('testdouble').reset();
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment