mocha: this.timeout() fails when using ES6's arrow functions

Thanks.

Why so many “magic” that, at the end, produces problems? Why not this?:

var mocha = require('mocha');

mocha.describe('foo', (suite) => {
  suite.timeout(100);

  suite.it('must love bar', () => ... );  
});

No globals, no problematic magic… but just JavaScript.

It binds the function to the test context, which can’t be done when using arrow functions. From http://mochajs.org/

Sorry about that!

I swear I read somewhere you could do something like this:

describe('a slow thing', () => {
 // test code...
}).timeout(5000);

which doesn’t alter the parameter contract to the provided function. Now I can’t find any reference to any such thing so maybe I just imagined it.

Since timeout is only relevant with done, why not simply attach the timeout function to the done function.

it('makes sense', done => {
    done.timeout(100);
});

@thom-nic It works on it, but not on describe.

describe('my test suite', () => {

  it('test case 1', () => {
    // ...
  }).timeout('10s');  // Works on `it`. This test case doesn't timeout

  it('test case 2', () => {
    // ...
  });  // This test case timeouts.

}).timeout('10s');  // Doesn't work on `describe`. Tests are already created when runs to here.

@mroien this is not a Mocha bug. the arrow syntax is not a 1:1 replacement for function. please read up on its limitations

I swear I read somewhere you could do something like this:

describe('a slow thing', () => {
 // test code...
}).timeout(5000);

which doesn’t alter the parameter contract to the provided function. Now I can’t find any reference to any such thing so maybe I just imagined it.

@thom-nic solution worked for me, thanks!

Something I’ve had in my mind for a while was being able to do:

it('...', (done) => {
  ...
  done()
})

and

it('...', (t) => {
  t.timeout(500)
  t.tag('integration', 'api')
  ...
  t.done()
})

using the same default interface.

Supporting both in the same default interface let’s you start using arrow functions in an existing codebase. Worth pointing out that the (done) syntax found in many tutorials online would still work, without flags or anything.

So in this implementation, you get as parameter the traditional done function, but with the utility functions added as properties of that done function object.

Anyone complaining about this doesn’t understand arrow functions.

Arrow functions are NOT a new fancy ES6 thing is supposed to replace the classic function () {}. The only purpose of arrow functions is that it inherits this from it’s parent, where the classic function () has it’s own this.

Yes, even when using full ES6 syntax, you should still be using function () if you want to use this in the correct context of your function. You should be using both function () and () => in your ES6 application depending on what you are trying to do.

this.timeout() doesn’t work with it('....', () => { ... }) because the callback is inheriting this from the parent describe() function, in which this.timeout() doesn’t make sense on that level.

For everyone still wondering about this, ensure you understand what an arrow function implies, then come back here and read on (there are plenty of resources out there that can explain this far better than I can).

The only way this arrow functions would work in this case is if we change the bdd API to pass a context object to every Runnable’s (hooks, tests) callback, instead of leveraging this. That’s not a bad idea, but it’s an earthquake of a breaking change, so will never happen. Instead of this:

it('should do something', function (done) {
  this.timeout(9000);
  // stuff
  done();
});

it’d look like this:

it('should do something', (context, done) => {
  context.timeout(9000);
  done();
});

That’d would break every async Mocha test in existence, regardless if it was backwards-compatible otherwise:

it('should do something', function (context, done) {
  // context is unused, but 'done' is now the second parameter
  this.timeout(9000);
  done();
});

We could provide an alternative bdd implementation that does this, however–it just wouldn’t be the default.

That’s about the most thorough explanation I have of “where this issue is at”. 😄

Although I am in love with arrow functions which are really useful for e.g. Array functions like .filter(i => i.val), what’s the problem with using normal functions? I think it’s quite useful to have describe and it globally so I don’t have to require them each time. Also since when is this magic, only because you don’t understand (arrow) functions? I definitely don’t want to provide a variable each time when I can return promises, otherwise I would have switched to something like ava a long time ago. Regarding the simplicity of mocha I think there shouldn’t be any large change on normal/arrow functions described in #1969. And please don’t tell me arrow functions are faster to type since your editor can transform a single f into function () {\n\t\n}.

@thom-nic , you may use the normal function form

describe('my test suite', function() {
this.timeout(n);

...
}

Hi Folks. I apologize for going dark after sparking the discussion back in august, i’ve been quite busy and actually have mostly completed/moved-on from that work.

I appreciate the detailed response about the different use cases and how its hard to dovetail them together. That was the most concise showing of the different setups mocha has to support that I’ve read. So thank you for your time on that one.

Looking back, its clear that I must have over-emphasized getting access to mocha context (this), when that aspect was really more of a convenient after-thought. I didn’t realize how easily it would draw attention away from what I was actually trying to do: which was having some fun by adding a temporal’ish extension (when) to the test dsl for streamlining one-off timeout adjustments (plus eliminating a common error for a particular style of tests, which I’ll explain below). Returning this was just another fun thing I thought of adding, the main idea (hence the name when) was for handling cases that needed different timeouts than normal.

Obviously, if I wanted to access the bound context I could simply use function directly as many have suggested, rather than hoisting it out with a wrapper. Thats not the issue. 😄 It isn’t lost on me how that might seem strange on the face of it. I’m hoping maybe it will round out the picture if I show how I was setting up some of the tests which led me down this path to begin with. To be clear, I’m not trying to sell any particular style here, use what works for you. This is what worked for me.

Ok First off, start with the assumption that we’re testing some setup that basically does one thing, but will do it for a wide range of inputs, and so we must test this thing out in a bunch of scenarios to ensure the outputs are correct. However, since the relevant application code “does one thing”, the underlying test procedure is pretty much always the same. I also don’t want to duplicate/mutate the test body needlessly, since that slows down how quickly I can add more test cases for new inputs and eventually the maintenance would become unreasonable.

So instead we write a function thats general enough to start the application code up with any potentially supported inputs, perform the test action, and then assert the results… Add that in my case I was working with the Promise abstraction (for reasons that i’ll leave out), so this generalized test procedure function naturally has to return that promise.then chain. Bread and butter es6 kind of stuff, so far so good.

Now comes the test scenarios, since we packed everything into our test-procedure function, the test cases are effectively defining the inputs and invoking the function.

So perhaps I write a bunch of tests like this, and everything seems to be working:

it('does something', function() {
  testProcedureFunction('something','1')
})

If you’re following along closely, you will have probably noticed already that this example has a bug. It is missing its return, and since testProcedureFunction is built on promises (pun intended), its always going to pass no matter whether the assertions at the end passes or fails. This is a bug, and it can be an extremely subtle one to track down sometimes. To illustrate, depending on how we wrote testProcedureFunction and how the application is written, lets say there is some synchronous code at the start, and that blows up instead of the end-of-test assertions, the testcase might even fail – leading us to think everything is fine.

The test should of course really look like this with a return:

it('does something', function() {
  return testProcedureFunction('something','1')
})

Now I know this test is going to be one line in most cases. Actually, every case will be one line, except for when the inputs are such that a larger timeout is required. Now among the differences between classical js functions and arrows, there is a particular aspect of arrow functions that is useful here: a single statement arrow function has an implied return when braces are omitted. If instead of writing a function {...}, a test uses the => ..., then I can easily scan those cases for the arrow and the lack of curly braces, and quickly infer that they cannot have this missing return issue, and it will take some extra steps (adding the braces back) for someone to mess that up.

Like so:

it('does something', () => testProcedureFunction('something','1'))

Now what if one of these cases takes longer than the others! We can of course set the timeout like this:

it('does something slow', function() {
  this.timeout(10000)
  return testProcedureFunction('somethingSlow','2')
})

Or maybe someone will make a mistake and do this first (which doesn’t work of course):

it('does something slow', () => {
  this.timeout(10000)
  return testProcedureFunction('somethingSlow','2')
})

But now we’re back where we started, the codebase has a pattern ripe for repeating, which is susceptible to the missing return bug (either by future me, or the next person to add a feature – the point is, its an easy mistake to make, it may go unnoticed, and it can be hard to track down). The when function solves this, and lets us use arrows consistently again:

it('does something slow', when(() => testProcedureFunction('somethingSlow','2'), 10000))

(note, i wasn’t able to get the .timeout(5000) dot-chaining suggestion above to work, may have been due to the version of mocha i needed to use, I don’t remember anymore, will give that a try!) (note2, notice that the use of when isn’t using the this parameter hoisting trick – it really was just an after-thought).

Maybe there are linters that can flag missing returns-for-promise bugs (or probably more realistically, enforcing a return statement with a rhs for every function). However that wasn’t an option at the time, plus I think the arrow syntax comes out shorter and I find it (subjectively/personally) easier to read and work with, which tipped the scales for me away from function.

So there you have it.

I don’t know if I’ll have time to respond again anytime soon, so I hope that was at least informative and clear, and maybe even puts some of the controversy surrounding the whole “access to mocha-context from arrows” stuff to bed.

Lastly, since I found function vs => confusing for a long time, I’ll drop this link in case its not clear to anyone casually reading why arrows can’t access this. It was the clearest explanation of functions vs arrows that I’ve found, and was what helped me finally understand the differences well enough to use them with full confidence.

https://hacks.mozilla.org/2015/06/es6-in-depth-arrow-functions/

i’m unclear, is there a solution to timeout out a before() call that uses arrow functions?

    before( async function () {
      data = await provider.getData();
      console.log(data);
      this.timeout(30*1000);
    });

Has no effect. still getting 4 second timeout here. This is the only slow thing in my test suite. I can put timeout to 30s in mocha.opts to solve the problem, but I don’t really need all tests to timeout after 30s just the one api call when 4s is fine for 99% of them.

Here’s how I solved it in the meantime (note that the first describe() uses function instead of fat arrow syntax:

describe('Search API', function () {
	this.timeout(30*1000);

	context('search', () => {
		let data;

		before( async () => {
		  data = await provider.getSearch();
		});

		it('returns results', () => {
		  expect(data).to.exist;
		  expect(data.results).to.be.instanceOf(Array);
		  expect(data.results.length).to.be.above(0);
		});
	})
});

We could provide an alternative bdd implementation that does this, however–it just wouldn’t be the default.

@boneskull A new bdd-es6 interface would be great 😃

Did anything come of this? I like the proposed solution, if only for my love of arrow functions and aversion to ‘this’ when it’s not required

@papercuptech The link is 404 not found.

@chovy You’re setting the timeout after the timeout occurs in await provider.getData(). Try using this instead:

before(async function () {
  this.timeout(30*1000); // set timeout first, then run the function
  data = await provider.getData();
  console.log(data);
});