Skip to content
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

Restrict modifier chaining #1670

Merged
merged 3 commits into from
Feb 4, 2018
Merged

Restrict modifier chaining #1670

merged 3 commits into from
Feb 4, 2018

Conversation

novemberborn
Copy link
Member

@novemberborn novemberborn commented Jan 29, 2018

Explicitly specify allowable chains, with some ground rules in mind:

Test chaining rules:

  • serial must come at the start
  • only and skip must come at the end
  • failing must come at the end, but can be followed by only and skip
  • only and skip cannot be chained together
  • no repeating

Hook chaining rules:

  • always comes immediately after "after hooks"
  • skip must come at the end
  • no only
  • no repeating

Additionally:

  • todo cannot be chained, except after serial
  • hooks are not available on serial

This commit also removes now unnecessary assertions from TestCollection.

Fixes #1182.


This allows the TypeScript definition to be simplified. I've done an initial refactor. Note that I've not included any JSDoc comments. I think we can do a lot better than we what we had before but I didn't want to sort that out just yet. Will open a follow-up ticket. I have fixed throws / notThrows taking observables. Would be nice to get a better type definition for Observable though. Will ask more explicitly in a follow-up ticket.

  • Create issue for Observable typing in TS and Flow
  • Create issue for embedding documentation in the TS and Flow definitions
  • Create issue in eslint-plugin-ava to check chains when linting

@novemberborn
Copy link
Member Author

Finished the TS refactor and did a similar one for Flow too. Also wrote some documentation for Flow, though still need to add the best way of stripping the type annotations. Will file a follow-up ticket for that too.

@novemberborn
Copy link
Member Author

I've documented how to strip flow type annotations and improved typings for t.throws() and t.notThrows(). TS should be fine without explicit type hints, though Flow needs some help. I've updated the documentation to reflect this.

lib/main.js Outdated
return Object.assign(test, runner.chain);
};

// Support ComonJS modules by exporting a test function that can be fully
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ComonJS => CommonJS

lib/main.js Outdated

// Support ComonJS modules by exporting a test function that can be fully
// chained. Also support ES module loaders by exporting __esModule and a
// default. Support `import {* as ava} from 'ava'` use cases by exporting a
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

import {* as ava} from 'ava' => import * as ava from 'ava'

@sindresorhus
Copy link
Member

Good to see this being cleaned up before the 1.0 release.

@sindresorhus
Copy link
Member

Can you open an issue on eslint-plugin-ava about making a rule for this?

@novemberborn
Copy link
Member Author

@sindresorhus Thanks. Will make the fixes and open follow-up issues. Will sneak in TypeScript 2.7 as well.

I'm considering revisiting the import {serial as test} / test.before() use-case in this PR. I think having accurate imports is more important than trying to "help". We should make serial with hooks work as expected too, and not support it before that's done.

Explicitly specify allowable chains, with some ground rules in mind:

Test chaining rules:

* `serial` must come at the start
* `only` and `skip` must come at the end
* `failing` must come at the end, but can be followed by `only` and `skip`
* `only` and `skip` cannot be chained together
* no repeating

Hook chaining rules:

* `always` comes immediately after "after hooks"
* `skip` must come at the end
* no `only`
* no repeating

Additionally:

* `todo` cannot be chained, except after `serial`
* hooks are not available on `serial`

This commit also removes now unnecessary assertions from TestCollection.

Fixes #1182.
Remove the need for a build step.

Update the `throws` and `notThrows` assertions, adding `Observable` and
allowing the returned error to be typed.

Remove the inline documentation, to be added later.

Simplify typing of `t.context`. Reassigning the `test` method along with
a type cast is now sufficient.

Update to TypeScript 2.7.1.
Minor syntactical changes aside, it's now equivalent to the TypeScript
definition!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Restrict test modifier chaining
2 participants