avajs / ava

Node.js test runner that lets you develop with confidence 🚀
MIT License
20.74k stars 1.41k forks source link

feature request: ava configs' factory function promise support. #1934

Closed iamstarkov closed 3 years ago

iamstarkov commented 6 years ago

Description

The config file must have a default export, using ES modules. It can either be a plain object or a factory function which returns a plain object.

vs

We highly recommend the use of async functions. They make asynchronous code concise and readable, and they implicitly return a promise so you don't have to.

I propose to add support for a promise from a factory function, because in my case i need to look around the environment I am going to run tests for and read some files. I can use synchronous versions of fs module, but it is contrary to node's guidelines and Ava's spirit.

Test Source

export default async ({ projectDir }) => {
  /* await stuff */
  return { /* … */};
}

Error Message & Stack Trace

☯ yarn test
yarn run v1.9.4
$ ./bin test

✖ Factory method exported by ava.config.js must not return a promise

Config

N/A, see above.

Command-Line Arguments

N/A

Relevant Links

Environment

Node.js v8.11.4
darwin 17.7.0
npm 5.6.0
yarn 1.9.4
ava 1.0.0-beta.8
novemberborn commented 6 years ago

Yup, we can do that. The restriction was added to reduce the complexity of adding this feature in the first place.

We'll have to make lib/cli work asynchronously:

https://github.com/avajs/ava/blob/bfe16300e07c12a1bad2e4f02898b1e7486daf70/lib/cli.js#L24

We'll also need to figure out how to make this work in our ESLint plugin:

https://github.com/avajs/eslint-plugin-ava/blob/6bb239fe1cd97fd671318253debc153fab7db071/util.js#L47

iamstarkov commented 6 years ago

sounds good

a-deeb commented 5 years ago

Hi, can help out and continue where things were left off, I just made a PR #2076

novemberborn commented 5 years ago

@a-deeb thanks, I'll have a look. If you've got questions about GitHub please comment on your PR here: https://github.com/avajs/ava/pull/2076 there's no need to close it and open a new one.

a-deeb commented 5 years ago

Thanks @novemberborn , if i have any questions ill ask here, and ill do my best to add this feature.

a-deeb commented 5 years ago

Sorry for opening and closing the PR I had opened before , I did too many commits on it , the changes i made kept failing the tests, to go back to previous version or to the original is the command git checkout -f or git reset --hard or git-revert ,Also I am trying to continue where @miguelsolano left off

novemberborn commented 5 years ago

@a-deeb don't worry about the commit history, we can fix that when we land the PR. There's many different ways of going back to a previous version. Perhaps give GitHub Desktop a try. They've got a post on it here: https://help.github.com/en/desktop/contributing-to-projects/reverting-a-commit

This post has some good information on pushing changes: https://www.atlassian.com/git/tutorials/syncing/git-push

I can't review your code if you keep closing the PR. Just leave #2077 open and let me know when you'd like me to take a look at it.

a-deeb commented 5 years ago

Thanks again @novemberborn !

novemberborn commented 5 years ago

Our ESLint plugin now relies on AVA's config, and the ESLint plugin must be synchronous. I'm not sure where that leaves us with supporting asynchronous factories.

iamstarkov commented 5 years ago

we still can support async config factories, but eslint plugin can be turned into synchronous with setTimeout or something

novemberborn commented 5 years ago

we still can support async config factories, but eslint plugin can be turned into synchronous with setTimeout or something

You can't go from asynchronous to synchronous.

iamstarkov commented 5 years ago

true, sorry, I had a brain fart

iamstarkov commented 5 years ago

I'll close it then

novemberborn commented 5 years ago

No I'd still like to support this. But we need to figure out the impact on our ESLint plugin. Maybe ESLint will support asynchronous plugins. Maybe, as long config-reliant rules are opt-in, we can make them fail when the config is asynchronous.

novemberborn commented 3 years ago

This will be available in AVA 4.