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

Support more test filename conventions #721

Merged

Conversation

kentcdodds
Copy link
Contributor

This PR just updates documentation. I couldn't for the life of me find out where this magic happens. Pointers? Thanks!

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @ariporad, @sotojuan and @Carnubak to be potential reviewers

@jamestalmage
Copy link
Contributor

Wait for #713 to land. Then check lib/ava-files

@@ -80,7 +80,7 @@ var cli = meow([
' ava --init foo.js',
'',
'Default patterns when no arguments:',
'test.js test-*.js test/**/*.js'
'test.js test-*.js test/**/*.js __tests__/**/*.js **/*.test.js'
Copy link
Member

Choose a reason for hiding this comment

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

I thought the convention is that __tests__ can be anywhere? Hence why it's wrapped in underscores.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, is my glob wrong? Should it be **/__tests__/**? That's what I did for my PR to nyc. Forgive my ignorance with glob 😅

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 **/__tests__/**

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated. 👍 I've subscribed to #713. As soon as that gets merged, I'll give this another look.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not __tests__/**/*.js?

@jfmengels
Copy link
Contributor

**/*.test.js ! I use this all the time, would love it if it landed in the default files ❤️
We'll need to update the linter too.

@kentcdodds kentcdodds force-pushed the pr/support-more-test-conventions branch from 7048016 to 1271010 Compare April 7, 2016 14:49
@kentcdodds
Copy link
Contributor Author

This has been updated 👍

@@ -17,7 +17,9 @@ function defaultIncludePatterns() {
return [
'test.js',
'test-*.js',
'test'
'test',
'**/__tests__/**',
Copy link
Member

Choose a reason for hiding this comment

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

Are all **/*.js files inside __tests__ tests? If so I think you just need to match **/__tests__ and AVA's built in directory recursion will take over.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

kentcdodds pushed a commit to kentcdodds/eslint-plugin-ava that referenced this pull request Apr 7, 2016
@kentcdodds
Copy link
Contributor Author

I imagine that we'll want to add a test for this, but I can't seem to find existing tests for the default inclusion glob...

kentcdodds pushed a commit to kentcdodds/eslint-plugin-ava that referenced this pull request Apr 7, 2016
kentcdodds pushed a commit to kentcdodds/eslint-plugin-ava that referenced this pull request Apr 7, 2016
@novemberborn
Copy link
Member

I imagine that we'll want to add a test for this, but I can't seem to find existing tests for the default inclusion glob...

Yea. I'm alluding to that in #736.

@@ -80,7 +80,7 @@ var cli = meow([
' ava --init foo.js',
'',
'Default patterns when no arguments:',
'test.js test-*.js test/**/*.js'
'test.js test-*.js test/**/*.js **/__tests__ **/*.test.js'
Copy link
Member

Choose a reason for hiding this comment

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

Documentation wise it should be **/__tests__/**/*.js, just like the test directory in ava-files is explained as test/**/*.js.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. Updated 👍

@novemberborn
Copy link
Member

LGTM!

@jamestalmage jamestalmage merged commit 9ceeb11 into avajs:master Apr 9, 2016
@kentcdodds kentcdodds deleted the pr/support-more-test-conventions branch April 9, 2016 03:56
@jfmengels
Copy link
Contributor

Woohoo! This makes my day, thanks @kentcdodds!

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

6 participants