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

tests failing #144

Closed
futagoza opened this issue Jun 27, 2015 · 10 comments
Closed

tests failing #144

futagoza opened this issue Jun 27, 2015 · 10 comments
Milestone

Comments

@futagoza
Copy link
Contributor

Implementation

Your new implementation of running the tests is okay if the module being tested isn't under some folder called node_modules some where in its pathname, but if it is, 0 tests are found.
It runs fine with this code: https://shrib.com/fs-extra-test.js-fix?v=pr

Copy, move and ncp

I'm not sure these other errors are related to me running on Windows 8.1 x64, but thought I should put them up: https://shrib.com/GsntR4KJ?v=pr

@jprichardson
Copy link
Owner

Your first point about the tests not running, I'm not clear about the case where they're not?

Regarding the tests failing, that's because you pulled the master branch (development). There's one issue that I believe to be a bug in io.js. More details here: #141.

@futagoza
Copy link
Contributor Author

Your tests.js file works if its located in C:/dev/node-fs-extra, finding 60+ tests, but fails with your code if the file is located in C:/dev/efe/node_modules/fs-extra, finding 0 tests.

In my edited file.js I have removed the line that looks for node_modules in the file path, also setting the walk method to look in ./lib only. This works fine and ensures that it wont look inside the modules own node_modules directory.

If its really required to search multiple folders to add tests to mocha, then you should wrap the walker (see example: https://shrib.com/fs-extra-test.js-modified?v=pr). This should avoid looking in folders that are in node_modules, unless of course there's a reason to scan 5000+ file names ;)

@futagoza
Copy link
Contributor Author

futagoza commented Jul 2, 2015

@jprichardson any update on this? Right now I've got a edited cloned repo with the changes I've mentioned above so that I can optionally run the fs-extra tests correctly (see above note about 'node_modules') along with mine

@jprichardson
Copy link
Owner

Sure, please submit a PR and I'll accept it. But note, it should find > 100 tests in either scenario.

@futagoza
Copy link
Contributor Author

futagoza commented Jul 2, 2015

Ok, I'll do that later on today when I get home

@jprichardson jprichardson modified the milestone: 1.0 Jul 2, 2015
@futagoza
Copy link
Contributor Author

futagoza commented Jul 3, 2015

Sorry for the delay, I got wrapped up in a problem.

The modified test.js is only finding 43 tests, I can't find the other 50+
Are you sure there are 100+ tests?

Also, I've sent the PR (#146). You can find another modified file.js at https://shrib.com/custom-fsextra-test.js?v=pr, this one being the one I used to count the tests.

@jprichardson
Copy link
Owner

The modified test.js is only finding 43 tests, I can't find the other 50+ Are you sure there are 100+ tests?

Yes :) https://travis-ci.org/jprichardson/node-fs-extra/jobs/69456709 Results from your pull request... you can see 115. I'm really curious to as why on your system there's only 43. If it's helpful I could attach an output from either spec or json and you could do a diff and see which ones aren't coming up...

@futagoza
Copy link
Contributor Author

futagoza commented Jul 3, 2015

oh lol, your talking about the individual registered tests aren't you? All this time I though you were talking about the files that contain the tests, which are only 43 files so far :D

@jprichardson
Copy link
Owner

Oh haha... that makes sense!

@jprichardson
Copy link
Owner

Closing as #146 fixes this. Thanks!

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

No branches or pull requests

2 participants