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

tap needs to be invoked differently on windows and linux #128

Closed
dotnetCarpenter opened this issue May 12, 2015 · 14 comments
Closed

tap needs to be invoked differently on windows and linux #128

dotnetCarpenter opened this issue May 12, 2015 · 14 comments

Comments

@dotnetCarpenter
Copy link
Contributor

tap needs to be invoked differently on windows and linux, which means that npm test fails to be cross-platform.
Example: in jfhbrook/node-ecstatic I can write "test": "tap test/" and it works on windows but not linux and the other way. I can write "tap test/*.js" and it works on linux but not in windows.

I even think it is a regression. I'm not 100% sure but I believe tap test/*.js used to work in Windows back in v0.4

@dotnetCarpenter
Copy link
Contributor Author

wow, didn't see that you pushed a new version. Just updated and it works fine in windows (even took care of the weird spawn error after the tests, I was seeing). Now I'm just waiting for confirmation on travis before closing this one.

@rmg
Copy link
Member

rmg commented May 12, 2015

#126 & #127 :-)

@dotnetCarpenter
Copy link
Contributor Author

@rmg thanks! I spend an hour trying to test this on travis. I've stared the yellow dot to death, finally gave up an made an issue report and then I see #126! Perfect dramatic curve ;)

@isaacs isaacs closed this as completed May 12, 2015
@isaacs
Copy link
Member

isaacs commented May 12, 2015

I think tap test/ should absolutely work on unix as well as Windows. Are there tests in the test/ folder? What happens?

@dotnetCarpenter
Copy link
Contributor Author

tap test/*.js works. I just updated tap from 0.4 to 1.0 and moved all .js files, that was not tests, into a sub directory. As you can see on travis, it is working. I got another issue with a missing file, that works on windows but not linux. I don't think it has anything to do with tap, though.

@dotnetCarpenter
Copy link
Contributor Author

and yes, tap test/ does also work on windows in v1.0.3. I was using v1.0.2 before

@dotnetCarpenter
Copy link
Contributor Author

hmm.. I just test on linux via cloud9 and tap test/ did not work, whereas tap test/*.js did.

@dotnetCarpenter
Copy link
Contributor Author

@isaacs here is the failed test case: https://travis-ci.org/jfhbrook/node-ecstatic/jobs/62297377
Hmm hang on. This might be something else. I think that tap test/*.js runs the test independently, so I can see the progress. While tap test/ runs everything before reporting back. So it might be failing tests that screw up tap.

dotnetCarpenter added a commit to dotnetCarpenter/node-ecstatic that referenced this issue May 12, 2015
@isaacs
Copy link
Member

isaacs commented May 12, 2015

Running tap test/ will recursively go through everything in the test/ folder running it with tap.

But in jfhbrook/node-ecstatic, you've got a TON of stuff in test/public/ that is not tests. It's broken symlinks and text files and other junk. So, yeah, running tap on that stuff breaks.

I suppose it could be made to skip over things at are not either js or executables, patch welcome.

@isaacs
Copy link
Member

isaacs commented May 12, 2015

Even then, this will always fail:

$ tap test/public/d.js
C!!!
total ................................................. 0/2


  0 passing (121.359ms)
  1 pending
  1 failing

So, the requirement would be for tap test/ --no-recurse or something.

@dotnetCarpenter
Copy link
Contributor Author

ahh ok. I only meant to run the test in the test/ directory. In v0.4, it seemingly worked. I just realized that it was giving false test numbers. Everything should be fixed in my latest commit, though.

The d.js broken symlink put me off for a while. Possibly a windows side-effect - not sure. But it was easily detectable and fixable once I got it on the cloud9 box.

@isaacs
Copy link
Member

isaacs commented May 12, 2015

@dotnetCarpenter Right, but how would tap know what is a test and what isn't? It just arbitrarily crawls all the dirs it encounters.

In this case tap test/*.js is absolutely the right thing to be doing. We just broke that on Windows, but it's fixed now.

@dotnetCarpenter
Copy link
Contributor Author

well, tap could look for tap.test. I'm not sure if it's worth the trouble to implement. Ractive does something like that with esprima but also uses regex for that in the component loader.

But I really think it's a documentation issue not a missing feature.

@isaacs
Copy link
Member

isaacs commented May 12, 2015

Yeah, I'm not gonna have the tap runner read tests and try to figure out if they're loading or calling tap. That's both too magical and a ton of work.

Besides, you might have something that outputs TAP format, but isn't actually loading reuquire('tap') to do it. And you don't need to call tap.test() any more, you can just do var t = require('tap'); t.pass('ok') and it'll work.

Doc patch welcome.

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

3 participants