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

Improve tests feedback, linked to #104 #105

Merged
merged 1 commit into from Jun 7, 2016

Conversation

jfmengels
Copy link
Contributor

@jfmengels jfmengels commented May 11, 2016

This is related to #104, and an attempt to improve the RuleTester's feedback. By default, it uses Mocha's describe and it to create rules, but it they are not defined, it uses the functions here.

I've managed to override the RuleTester test creation a bit to use AVA's test rather than Mocha's it, and here are the results:

Before

When run with Mocha

image

When run with AVA (with --verbose)

image

When run with AVA and tap-mocha-reporter

image

The odd test.xxx(t => {}) kind of everywhere are the second part of the title (since it takes as title the code, and the code has a \n in the middle). If we remove the \n, it gets much clearer
image

Please check out eslint-ava-rule-tester, which exports a modified RuleTester. I'm up for modifying the name, but it's already published (could not make it all work with npm link), which contains the same code that you can see in test/helpers/ava-rule-tester.js (I kept it ll so that you guys could easily see what I did here).

Let me know what you guys think.

PS: Obviously this is just a draft, I haven't migrated all the tests and have even left an error so that you can see the error feedback more easily.

@sindresorhus
Copy link
Member

Nice! Looks so much better now :)

@sindresorhus
Copy link
Member

Maybe we should do a PR to ESLint adding a hook to their rule checker so any test framework could easily hook into it?

@jfmengels
Copy link
Contributor Author

Nice! Looks so much better now :)

Yeah!

Maybe we should do a PR to ESLint adding a hook to their rule checker so any test framework could easily hook into it?

Sure. By their contribution guidelines, we should first create an issue though. Not sure I see what the hook should look like though?

@jfmengels
Copy link
Contributor Author

By the way: Found this comment eslint/eslint#5040 (comment), where they talk about eventually using AVA under the hood for RuleTester.

@jfmengels
Copy link
Contributor Author

Created an issue in ESLint about this eslint/eslint#6227

@jfmengels
Copy link
Contributor Author

Now that we know that ESLint considers this okay and describe and it as part of their API (meaning it will be enforced by semver), shall I go ahead with finishing this? If so, I'll go ahead and do eslint-plugin-xo too (and my own plugins).

@jfmengels
Copy link
Contributor Author

jfmengels commented Jun 4, 2016

Before

image

After

image

Biggest number of test additions you'll ever see in one commit :D
(Lots of diff, but it's mostly just de-indenting all the tests)

@jfmengels
Copy link
Contributor Author

jfmengels commented Jun 4, 2016

Hmm, looking into the tests a bit later today (worked on my end before I reinstalled all node_modules). Stay tuned.

@sindresorhus
Copy link
Member

shall I go ahead with finishing this?

Let's do it! :D

@jfmengels
Copy link
Contributor Author

There you go :)

@sindresorhus sindresorhus merged commit 531fc30 into master Jun 7, 2016
@sindresorhus sindresorhus deleted the improve-tests-feedback branch June 7, 2016 10:36
@sindresorhus
Copy link
Member

Yay! So much better output :)

leo-excited

@jfmengels
Copy link
Contributor Author

jfmengels commented Jun 7, 2016

:)

Will do a PR for XO soon-ish.

@jfmengels
Copy link
Contributor Author

PS: eslint/eslint#6331, if you have comments to improve this ;)

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.

None yet

2 participants