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

Add -i flag to ignore tests inside node_module directories #451

Merged
merged 1 commit into from Jan 8, 2020

Conversation

@kepikoi
Copy link

kepikoi commented Dec 13, 2018

This will partly resolve #428 by adding the -i flag to ignore any tests inside node_module directories to allow co-locating tests with their modules. Running tape **/*.spec.js -i in the project directory won't execute tests inside its npm packages that may break the tape run due different testing frameworks etc.

@kepikoi

This comment has been minimized.

Copy link
Author

kepikoi commented Dec 13, 2018

Sorry, forgot to change the branch and 2fc5d1f 0ba37b0 creeped accidentally in. I undid these changes in the following commits

Copy link
Collaborator

ljharb left a comment

See #428 (comment).

I don’t think that it makes sense that “-i” would ignore node_modules, for one - even if “respecting gitignore” wasn’t a better option. Can you update this PR to implement that?

@kepikoi kepikoi force-pushed the kepikoi:feature-ignore-flag branch from 07fe9b6 to 3f983fb Dec 14, 2018
@kepikoi

This comment has been minimized.

Copy link
Author

kepikoi commented Dec 15, 2018

I'm not quite sure - do you want me to remove the i flag and make it ignore node_modules by default? Or are you saying the i flag makes only sense when excluding .gitignore entries is implemented?

@ljharb

This comment has been minimized.

Copy link
Collaborator

ljharb commented Dec 15, 2018

I think I’m saying a few things:

  1. a generic “ignore” flag would have to take explicit paths, or a path to an ignore file, and not default to “node_modules”
  2. the root issue is that tape is including paths it shouldn’t be testing - namely, anything not committee to the repo. node_modules is just a symptom of this.
  3. I think excluding gitignored files (under an option) is the best use of time and effort to address this problem

Are you willing to update this PR to implement #428 (comment) ?

@kepikoi

This comment has been minimized.

Copy link
Author

kepikoi commented Dec 15, 2018

Ok, I will update the pr to exclude gitignored files when using -i and let it ignore anything under node_modules by default.

Copy link
Collaborator

ljharb left a comment

To clarify: "under node_modules by default" is not acceptable; the functionality that we should be shooting for is "ignoring things that are gitignored" - which should include node_modules.

bin/tape Outdated
if (module) {
/* This check ensures we ignore `-r ""`, trailing `-r`, or
* other silly things the user might (inadvertently) be doing.
*/
require(resolveModule(module, { basedir: cwd }));

This comment has been minimized.

Copy link
@ljharb

ljharb Dec 19, 2018

Collaborator

please revert this spacing change (and the others)

@@ -84,6 +84,14 @@ $ tape -r ./my/local/module tests/**/*.js

Please note that all modules loaded using the `-r` flag will run *before* any tests, regardless of when they are specified. For example, `tape -r a b -r c` will actually load `a` and `c` *before* loading `b`, since they are flagged as required modules.

## Co-locating tests

When co-locating tests with their modules adding the `-i` flag will allow to ignore any tests that reside inside /node_modules/ directories. This will allow tape processing tests recursively in the project:

This comment has been minimized.

Copy link
@ljharb

ljharb Dec 19, 2018

Collaborator

I don't recommend colocation at all, so I don't want that mentioned in the readme.

@r0mflip

This comment has been minimized.

Copy link
Contributor

r0mflip commented Jun 18, 2019

+1 for "ignoring things that are gitignored"

@ljharb

This comment has been minimized.

Copy link
Collaborator

ljharb commented Dec 27, 2019

@kepikoi are you interested in completing this PR?

@kepikoi

This comment has been minimized.

Copy link
Author

kepikoi commented Dec 27, 2019

Sorry, no capacity rn for ignoring things that are gitignored. Closing the PR

@kepikoi kepikoi closed this Dec 27, 2019
@ljharb

This comment has been minimized.

Copy link
Collaborator

ljharb commented Jan 8, 2020

Replacing this with #492.

@ljharb ljharb force-pushed the kepikoi:feature-ignore-flag branch from c89e7e7 to 3aa123c Jan 8, 2020
@ljharb
ljharb approved these changes Jan 8, 2020
@ljharb ljharb force-pushed the kepikoi:feature-ignore-flag branch from 3aa123c to a0f9350 Jan 8, 2020
@ljharb ljharb closed this in #492 Jan 8, 2020
@ljharb ljharb merged commit a0f9350 into substack:master Jan 8, 2020
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@kepikoi kepikoi deleted the kepikoi:feature-ignore-flag branch Jan 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.