Tape: number of tests do not match up

[tcurdt]

tape -- tests/*.js core/tests/*.js plugins/*/tests/*.js
# tests 115

and

tape -- tests/*.js
# tests 34

tape -- core/tests/*.js
# tests 89

tape -- plugins/*/tests/*.js
# tests 81

34 + 89 + 81 = 204

115 != 204 WTF?

[ljharb]

Can you make a gist with the files in question?

[tcurdt]

I can do you one better - here is the full project https://github.com/tcurdt/xstatic

[tcurdt]

This is how my node_modules looks like

@tcurdt
babel-core
babel-plugin-transform-react-jsx
blue-tape
front-matter
handlebars
highlight.js
less
libxmljs
marked
minimatch
moment
node-sass
nunjucks
tape
xmlbuilder
xstatic-atom -> ../plugins/atom
xstatic-babel -> ../plugins/babel
xstatic-core -> ../core
xstatic-css -> ../plugins/css
xstatic-epub -> ../plugins/epub
xstatic-filter -> ../plugins/filter
xstatic-frontmatter -> ../plugins/frontmatter
xstatic-handlebars -> ../plugins/handlebars
xstatic-json -> ../plugins/json
xstatic-less -> ../plugins/less
xstatic-markdown -> ../plugins/markdown
xstatic-merge -> ../plugins/merge
xstatic-nunjucks -> ../plugins/nunjucks
xstatic-sass -> ../plugins/sass
xstatic-sitemap -> ../plugins/sitemap
xstatic-sort -> ../plugins/sort

[ljharb]

On a cloned repo, after npm install, for the first example, I get 204 tests run.

[tcurdt]

Hm. That's fine.

For development I've used npm link - could that lead to problems?

[ljharb]

Possibly - what are you linking exactly?

[tcurdt]

@ljharb see above - all the xstatic-* modules

[ljharb]

That absolutely could cause issues. npm link is meant to be used for separate modules - not to create symlinks within your primary module to avoid the perceived inconvenience of using "../"-based relative paths.

Closing this for now, since I can't reproduce - I don't think the tape runner needs to be following symlinks here.

[tcurdt]

These are separate modules. I didn't use npm link to avoid "../" but to work on the latest version of modules (which is whattnpm link is meant for to my understanding). Not sure I can follow your argument there.

Reproducing this should be easy. Just rm -rf xstatic-<you pick one> and then use npm link to set it to the latest version.

[ljharb]

@tcurdt i may have misunderstood - you said you have xstatic-atom -> ../plugins/atom for example, and the implication was you're running tests inside plugins/atom from the same project that xstatic-atom is linked in?

[tcurdt]

@ljharb since the xstatic project is early stage I have all plugins and the core in one repo (like react and ember are doing it as well). The core and the plugins are all individual modules though (with own package.json files).

At the top level I am running some integration tests (which basically just another project linking to all the other modules). So best to forget all is in the same repo - that doesn't really matter. The ../ is merely a coincident.

tape -- tests/*.js core/tests/*.js plugins/*/tests/*.js | faucet

is run on the top level. Which seems to work OK with a fresh install. When I use npm link though (to use the latest versions for development) I am seeing the reported behaviour.

Does that make more sense now?

[ljharb]

Hmm. It's likely that the core/tests/*.js are being skipped - if you run tape -- tests/*.js plugins/*/tests/*.js | faucet do you still get the same 115 tests?

[tcurdt]

Yes, tape -- tests/*.js plugins/*/tests/*.js | faucet gives me 115 tests.

[ljharb]

OK - then since core itself is a symlink (and plugins/foo is, but plugins is not) i suspect it's that the tape runner doesn't traverse symlinks. if you put core inside a dir, my hunch is that you'd get all 204. @tcurdt, can you confirm?

[tcurdt]

Not sure I can follow. You think foo/core should work?

[ljharb]

@tcurdt i think that if the first "item" in one of the paths you pass to tape is a symlink, it will break, but otherwise, it will work

[tcurdt]

@ljharb but none of those are symlinks - the symlinks are only in node_modules

But I am seeing even more strangeness.

tape -- plugins/sass/tests/*.js

works fine when called like above, now when it's part of match

tape -- plugins/*/tests/*.js

it fails.

WTF? Any idea?
(This may or may not be related)

[ljharb]

I'm going to reopen this for now - it would help if you made a branch on the repo that had the symlinks committed to it was easier to repro.

[tcurdt]

This seems to be an unrelated issue - but try the following:

git clone [email protected]:tcurdt/xstatic.git && cd xstatic
git checkout tc-testing
npm i
sh deps.sh

then try (1)

tape -- plugins/sass/tests/*.js

and then (2)

tape -- plugins/*/tests/*.js

You will find that the test worked in (1) but fails in (2).

I'll try to make the linking easily reproducible as well.

[tcurdt]

If you run links.sh it will create the setup I had.

tape -- core/tests/*.js # tests 89
tape -- plugins/*/tests/*.js # tests 81
tape -- tests/*.js # tests 34
tape -- tests/*.js core/tests/*.js plugins/*/tests/*.js # tests 115
tape -- core/tests/*.js plugins/*/tests/*.js tests/*.js # tests 89

[ljharb]

wow, lots of links :-) k, i've reproduced it locally - thanks!

I also see:

tape -- plugins/*/tests/*.js tests/*.js core/tests/*.js # tests 115
tape -- plugins/*/tests/*.js core/tests/*.js tests/*.js # tests 115
tape -- plugins/*/tests/*.js tests/*.js # tests 115
tape -- tests/*.js plugins/*/tests/*.js # tests 115
tape -- core/tests/*.js tests/*.js plugins/*/tests/*.js # tests 89
tape -- core/tests/*.js tests/*.js # tests 89
tape -- core/tests/*.js plugins/*/tests/*.js # tests 89
tape -- tests/*.js core/tests/*.js # tests 34
tape -- tests/{behaviour-nop,it-blog}.js core/tests/{changes,collection,context,glob,lazy,project}.js # tests 34
tape -- tests/it-blog.js core/tests/*.js # tests 30
tape -- tests/behaviour-nop.js core/tests/{changes,collection,context,glob,lazy,project}.js # tests 84
tape -- tests/behaviour-nop.js core/tests/{changes,collection,context,glob,lazy,project}.js # tests 4 (but i see results from 84)

very weird. I'm not sure how to solve this nor why it would be happening. The fact that it requires all those links suggests it's a symlink traversal problem.

[tcurdt]

I now have all the tests running from the parent project. For head development I use a script that replaces the deps with straight symlinks. That works around the problem (and is much faster).

The original problem is mega odd though.
I am wondering if this could be related to the use of blue-tape.

[ljharb]

That is possible. it would be useful to be able to repro it without blue-tape.

[brendanlong]

Do you need to use glob's "follow" option?

[ljharb]

I tried that, but it didn't seem to fix the problem.

[LongLiveCHIEF]

I've just found that I'm unable to test more than one dir using glob expansion, and I think it's another indicator of this issue.

For example:

tape ./test/**/*.spec.js is the command I'm running, against:

-/test
|- example.spec.js
|---/core
       |-componentA.spec.js
       |-componentB.spec.js
       |--/sub-core
             |- componentC.spec.js

Only tests from the /core folder however, were run.

I created a test glob.js file, and gave it the same ./test/**/*.spec.js pattern, and it logged out all 4 files.

ar glob = require("glob");

glob("./test/**/*.spec.js", function(er, files){

    files.forEach(function(file){
        process.stdout.write(`matched file: ${file}\r\n`);  
    });
});

Here's the commit from my project demoing this.

I think the problem has to be related to how the results from glob are passed to tap?

[LongLiveCHIEF]

I think the problem is in the bin/tape file. I don't think the glob is correctly assigned to opts._ when any cli option is given, or whenever -- is used, in combination with being run from an npm-script.

[LongLiveCHIEF]

I'm going to create a test/bin.js file, and see if I can write some test conditions that should pass, but fail, and then go from there. I suspect that we'll need to add {--:true} to the parser.

[LongLiveCHIEF]

Ok, definitely some odd things going on here, and I think it's because minimist is parsing the globs. I created the following changes to the bin/tape file in my LongLiveCHIEF/tape fix-271 branch, in order to first get an idea of what exactly is being parsed into opts._.

I expected the globs to be passed in, however, in each case, the glob's have already been matched. If you want, you can pull the changes in my fork into your local clone of substack/tape, then run npm link from within your tape module.

Then, you can go to any other project you are running tests using tape, and run npm link tape. Now when you execute tape, it will create a test-matches.txt file in the cwd.

As I continue to look into this, I wanted you guys to be able to look at what I'm seeing with these early findings. Very interesting!

[LongLiveCHIEF]

Here's another quirk. I'm running:

tape -r babel-register -r ./test/common.js ./test/**/*.spec.js

When I run this using /bin/bash, or execute using npm test, it matches only 2 of the files given in my example above. However, when I use /bin/zsh, it matches all 4 correctly.

[LongLiveCHIEF]

@tcurdt @ljharb time to let me know about your environment conditions:

• What shell were you using?
• What version of node are you running?
• Were you running the tape command via npm test?

[LongLiveCHIEF]

Looks like node itself is expanding the glob's as they're assigned to process.argv. If you put the glob in a string, everything works perfectly:

Bad:
tape ./test/*.js

Good:
tape './test/*.js'

[tcurdt]

Interesting findings, @LongLiveCHIEF

1. shell: GNU bash, version 3.2.57(1)-release (x86_64-apple-darwin15)
2. node: was 4.3.x or 4.4.x
3. I have it setup for npm but I tried both ways

[ljharb]

@LongLiveCHIEF putting the globs in quotes is always a requirement, or else the shell will expand it for you. This isn't unique to tape, or glob, but is part of any command line tool.

@tcurdt does everything work for you if you pass the file path in quotes?

[LongLiveCHIEF]

Bug Confirmed

Cause

This bug occurs depending on whether or not your shell supports recursive globbing. By the time the values in process.argv are able to be parsed, the glob's passed have already been expanded into matches files, which is done by your shell.

Resolution Options

1. create a subproc and pipe everything after tape in the node process call into stdin for the subproc (as a single string), and then parse _that_ with minimist instead.
2. Just update all the usage docs to ensure the glob pattern uses quotes around the glob to be matched.

Temporary Workaround

In the short term, you should just be able to encase your glob's in strings in order to get the right number of tests to run. Examples:

From the command line:

$ tape './test/**/*.js'

Or using npm:

// package.json
{
    "scripts":{
        "test": 'tape "./test**/*.js"'
    }
}

// run using `npm test`

(/cc @tcurdt )

Input Needed

I'd like a project owner/maintainer to chime in on what direction they'd prefer so I know which route to go with my PR.

I checked out mocha, and it looks like they get the usability without the string encasing by going with the first option.

(/cc @ljharb )

[ljharb]

Certainly option 2 seems simplest, and is what most command line tools require.

However, I'd love to see what a PR would look like for option 1 (even though I think we should always recommend quoting path arguments in all cases).

[LongLiveCHIEF]

I think it would add about 40-60 loc to the /bin directory, but it would be a non-breaking change.

I think option 2 is certainly easier. I _thought_ it was fairly standard practice to support globbing without string encasing, but after digging around a bit, it's 50/50.

If we go with option 2, I think we should update the README to mimic the [eslint.org cli-usage docs](http://eslint.org/docs/user-guide/command-line-interface homepage, up through the part where it says:

Please note that when passing a glob as a parameter, it will be expanded by your shell. The results of the expansion can vary depending on your shell, and its configuration. If you want to use node glob syntax, you have to quote your parameter (using double quotes if you need it to run in Windows), as follows

[LongLiveCHIEF]

Looks like all this time, the Usage section of the README.md had us covered.

@tcurdt can you confirm that encasing your glob's as strings solves your problem, and then we can close?

[tcurdt]

@ljharb I changed the setup and get by without the linking - and with that all works as is.
@LongLiveCHIEF Frankly speaking - I don't get it. If quoting is the problem/solution - why the different counts? The expansion should be the same for the combined and the three separate calls to tape.

[LongLiveCHIEF]

Can I ask... why are you using -- in your command execution?

[tcurdt]

@LongLiveCHIEF could that be a problem?

I guess that's a leftover from the npm setup where I use it with istanbul

istanbul test tape -- tests/*.js core/tests/*.js plugins/*/tests/*.js | faucet

[ljharb]

I don't think that should be a problem. However, they definitely need to be quoted.

I think I'm going to close this for now.

If someone wants to try a PR that makes unquoted options work, that'd be very interesting to consider.