Skip to content

Support assertion message in t.timeout() #2443

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

Closed
shellscape opened this issue Apr 6, 2020 · 12 comments · Fixed by #2524
Closed

Support assertion message in t.timeout() #2443

shellscape opened this issue Apr 6, 2020 · 12 comments · Fixed by #2524
Assignees
Labels

Comments

@shellscape
Copy link

Please provide details about:

  • What you're trying to do

I'd like to display a message to folks observing test runs on a particular timeout to indicate that they need to check some configuration locally. This particular scenario arises from bad user config that is detectable only at runtime (thanks AWS).

  • Why you can't use AVA for this

I've sifted through the docs and can't find a good way to do this. I haven't taken a look at the source for a hack as I've been burned by doing that in the past and upgrading major versions.

  • And maybe how you think AVA could handle this

Spitballing here, but perhaps:

test('verify email token', async (t) => {
  t.timeout(2000, 'Please check yo stuffs!');
  const result = await longRunningTask();
  t.assert(result);
});

// or if this is something you all would prefer to indicate for an entire suite

test.timeout(2000, 'Please check all the things');
@novemberborn

This comment has been minimized.

@shellscape

This comment has been minimized.

@novemberborn

This comment has been minimized.

@shellscape

This comment has been minimized.

@novemberborn novemberborn changed the title Feature: timeout message (or hook) Support assertion message in t.timeout() Apr 10, 2020
@novemberborn novemberborn added enhancement new functionality help wanted and removed question labels Apr 10, 2020
@novemberborn
Copy link
Member

We can support an assertion message in t.timeout().

This requires adding an argument here:

this.timeout = ms => {

And then passing it along to here:

ava/lib/test.js

Line 435 in 447d371

this.saveFirstError(new Error('Test timeout exceeded'));

We should retain the default message. I think that should just show up correctly in the reporters.

We must also add it to the type definition:

ava/index.d.ts

Line 344 in 447d371

(ms: number): void;

Our ESLint plugin must also be updated:

https://github.com/avajs/eslint-plugin-ava/blob/bf2364459d240684b15b57f2542abc47f623c2d8/rules/assertion-arguments.js#L77

@novemberborn novemberborn reopened this Apr 10, 2020
@shellscape
Copy link
Author

Fantastic. I'm happy to help with that work if you'd like it

@novemberborn
Copy link
Member

@shellscape yes, PRs welcome 😉

@shellscape
Copy link
Author

Checking in, this is still on my radar.

@jonathansamines
Copy link
Contributor

@novemberborn I'd like to work on this, if no one else is already doing so.

@novemberborn
Copy link
Member

@jonathansamines yes that'd be great. I assume @shellscape hasn't found the time to pick this up yet.

@shellscape
Copy link
Author

You have assumed correctly. 🙇

@novemberborn
Copy link
Member

@shellscape no worries! Hope all is well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants