Add support for concurrent test runners (ava)?

[paultyng]

Perhaps instead of the common pattern of:

afterEach(() => td.reset());

An additional pattern similar to:

beforeEach(t => {
  t.context.td = td.scoped(); // or new TestDouble() or ...?
});

test(t => {
  const { td } = t.context;

  td.verify(...);
});

Could be supported?

[paultyng]

I'm not 100% sure it doesn't work, I've done some small samples with testdouble and ava and didn't hit any issues but probably because my tests didn't overlap very much. I'll have to see if I can reproduce an error in a test suite.

[searls]

Let's close for now until someone somewhere runs into a real (as opposed to speculative) problem.

[ackerdev]

Here's an example failing test case where ava concurrency clashes with td.replace / td.reset

const test = require('ava');
const td = require('testdouble');
const foo = {
  bar: function() {
    return this.baz()
  },
  baz: function() {
    return 'baz';
  }
};

test.beforeEach(t => {
  td.replace(foo, 'baz');
});

test.afterEach(t => {
  td.reset();
});

test('async delay', t => {
  t.plan(1);
  td.when(foo.baz()).thenReturn('fake');
  return new Promise((resolve, reject) => {
    setTimeout(resolve(), 1000);
  })
  .then(() => {
    t.is(foo.bar(), 'fake');
  });
});

test('accidentally overwrites stub', t => {
  t.plan(1);
  td.when(foo.baz()).thenReturn('faketoo');
  t.is(foo.bar(), 'faketoo');
});

When foo.bar() executes in the async delay test, it will return undefined. Behind the scenes it seems td.replace is happily replacing already-replaced object properties; so we end up with a replaced-replacement, which gets a stubbing written which is quickly overwritten, and then when reset it 'unwraps' the outer replacement leaving us with a fresh replacement that has no stubbings defined.

[searls]

Thanks @ackerdev -- that gives me some insight to think on how (and whether) to undertake what we'd need to do to support parallel tests

[searls]

I've given more thought to this and I still don't have any good proposals. My thoughts follow.

Global state

There are several aspects of td's API design that rely on global state:

• td.replace(obj, 'someProp') - which will actually replace obj.someProp from the time it's called to the time td.reset() is next invoked. If two tests run concurrently and obj is referenced by both, these replacements will step on one another
• td.replace('../path/to/module') - td's quibble integration will intercept all calls to require for the given absolute module path until reset is called
• td.when(myTd()).thenReturn('foo') - configured stubbings are stored globally for a given test double function, since there's only one stubbing store, and it's keyed off the identity of test double functions.
• td.verify(saveThing('thing')) - recorded invocations are stored globally and are also keyed off the identity of test double functions

So the first two instances are examples of td.js mutating state of things it doesn't own, and the final two describe td.js's internal storage of the state of known test double functions.

What can we do?

If the question is "how do we use td.js with a concurrent test runner?", the answer must be in two parts:

1. Don't use any type of td.replace on modules or objects that are shared across tests. There's just no safe way to muck with global state and nothing the library can do to safeguard this
2. Sequester the config & call stores for td.when/td.verify between tests, by either:
   1. instruct users to separate test double instantiation for each test so that two test methods aren't sharing the exact same test double function; this should be default/normal practice regardless of whether you're using a concurrent runner or not (e.g. create them in a beforeEach, not in top-level scope or in a before (all))
   2. if for some reason the above isn't tenable (I can't imagine it wouldn't be), td could implement a feature akin to sinon.sandbox() that forked its state (e.g. td2 = td.fork()) to keep these things separate; I really don't think this can possibly be necessary, though, if each test is creating its own test doubles, as it should be

Ava-specific advice

(FWIW, this has been raised with respect to Sinon as well)

Now, one thing Ava does that is interesting is that it forks each test file into a separate operating system process. If every single test were split into its own process, all these problems would go away, since there'd be a separate global state for each.

Thinking more about Ava-specific fixes to this issue, these seem like one's options:

• You can use .serial and make any tests using td run serially inside of a specific file. This won't please purists, but consider that any test using test doubles:
  • Should be super isolated and free of I/O or anything that would make them benefit from concurrency
  • Most often ought to be able to be defined synchronously
  • Would have no way to do anything like td.replace without assurance it's being run serially when in the context of a single process
• You could break every test into its own file, which seems like a lot to manage
• You could contribute to Ava, where a "fork per test" issue has already been discussed and de-prioritized in Run individual tests in separate processes avajs/ava#421

I think the above Ava-specific options are all worth thinking about, but I don't think there's anything the library can do to solve the td.replace problem and anything it should do to add sandbox/fork support, unless I'm really missing something (counter-examples welcome)

[kristianmandrup]

See https://www.npmjs.com/package/forking-tap for inspiration ;)