Add idle timeout support

[MarkTiedemann]

Just switched to AVA -- and I love it! -- but, -- and this is really a minor issue -- I also noticed that I kept reusing code like this:

import test from 'ava';

function failAfter(time, t){
  setTimeout(() => {
    t.fail('timed out after ' + time + ' ms');
    t.end(); /* fail fast! */
  }, time);
}

async function thisShouldntTakeLong(){ /* or does it? >_< */
  return new Promise(resolve => setTimeout(resolve, 9999));
}

test('this shouldnt take long', async t => {
  failAfter(1000, t);
  await thisShouldntTakeLong();
});

And just to be clear, I don't like global test timeouts -- I think they're a terrible idea.

Yet again, I think something like t.failAfter(time) is not too bad.

What do you guys think? :)

[jamestalmage]

I don't like global test timeouts -- I think they're a terrible idea.

Why?

[MarkTiedemann]

@jamestalmage Well, I'm just kinda annoyed of having to change the default timeout in other test frameworks again and again.

I'm actually fine with having a global timeout, as long as it is disabled by default.

[jamestalmage]

Right now AVA hangs forever if you fail to call t.end(). That's no good. Especially if one of those tests make it to your CI server.

One thing I am always writing (in mocha) is:

if (process.env.TRAVIS) {
   this.timeout(fiveTimesWhateverItTakesLocally) 
}

I dislike having to wait to see my mistake during development, just to accommodate the slower CPU on my CI. It would be cool to build that in to AVA (though maybe not possible to do in a reliable way across all CI implementations out there).

Either way, I think AVA needs global and per tests timeouts with sensible defaults.

[sindresorhus]

I think AVA needs global and per tests timeouts with sensible defaults.

What are sensible defaults in your opinion?

[MarkTiedemann]

@jamestalmage You changed my mind. :) I agree, in CI defaults do make sense.

[jamestalmage]

Ooh. That is a hard question. 😄

Locally, I would prefer something really fast. 500 ms or something. Some users might find that surprising (though it might also help to gently remind people that they should be writing small, fast tests). It may also unfairly inconvenience users with really old/slow CPU's.

[bookercodes]

Hi,

I gathered some data on the default timeouts used by other popular tools. Hopefully it will aid you in reasoning about a "sensible default":

Tool	Default timeout for async tests	Source
Mocha	2000ms (2s)	source
Jasmine	5000ms (5s)	source
Node Tap	30000ms (30s)*	source
Tape	None	N/A

* 240000ms (240s) if __coverage__ global is truthy.

As an aside, I think there should be a command-line argument and/or environment variable to configure the timeout value. This'll be useful when running tests in a slow environment.

[markthethomas]

@jamestalmage @alexbooker I agree w/ the need for global/local timeouts and for me a faster default would be preferable and, to your point, encourage me to write smaller, faster tests 👍 Maybe it's crazy/overkill, but what about using something like os.cpus() and the speed prop to smart-set the timeout? I know estimating cpu perf is tough, but just a thought I had :)

[MarkTiedemann]

500 ms seems reasonable to me since AVA is supposed to be fast. And it's also supposed to be minimal; that's why I think timeouts should be disabled by default.

@markthethomas - I don't think smart-setting the timeout based on CPU information is helpful. For normal tests, you don't really need timeouts anyway. At least, when I implement timeouts, it's usually because of possible network issues, like "if you can't connect to service X within Y seconds, abort the tests".

[markthethomas]

@MarkTiedemann sounds good to me — just a thought I had :)

[vadimdemedes]

@markthethomas AVA is supposed to be fast, but that has nothing to do with user's tests. 500 ms is too small imho, I think we'd better choose tap's default timeout - 30 seconds.

[markthethomas]

Sorry, the "sounds good" was in reference to using the os module :)

Mark

Sent from my iPhone

@markthethomas everywhere

On Nov 9, 2015, at 3:42 PM, Vadim Demedes notifications@github.com wrote:

@markthethomas AVA is supposed to be fast, but that has nothing to do with user's tests. 500 ms is too small imho, I think we'd better choose tap's default timeout - 30 seconds.

—
Reply to this email directly or view it on GitHub.

[MarkTiedemann]

500 ms seems reasonable if you want to enforce small, fast local tests. 30 s seems reasonable if you just want to prevent CI from hanging.

So both defaults kinda make sense, in their own narrow context. So actually they don't: Because how do you know the context? You don't know.

But the solution is simple, I think. If the timeout is disabled by default, we don't need to pick a pseudo-reasonable default -- just let the developer decide!

If someone wants to run quick AVA tests locally, let him use a timeout option:

--timeout 500

And if someone just wants to prevent CI from hanging, let him set his own reasonable timeout:

// timeout still disabled
if (process.env.TRAVIS) {
  test.setTimeout(30000); // timeout now enabled!
}

How about this idea? :)

[jamestalmage]

I think both cli and api hooks are a given. We are all just bikeshedding over what the defaults will be.