search

thlorenz / proxyquireify

browserify >= v2 version of proxyquire. Mocks out browserify's require to allow stubbing out dependencies while testing.
MIT License
151 stars 24 forks source link

Use fill-keys to copy properties from source to stub #33

Closed bendrucker closed 9 years ago

bendrucker commented 9 years ago

Closes #32

Ready for review @thlorenz

thlorenz commented 9 years ago

LGTM, nice work! Is the plan to use fill-keys in proxyquire as well to get consistent behavior? That'd be good. Unless it's consistent now in which case we could defer updating proxyquire until we are about to make a change to either fill-keys implementation.

Please merge, I'll publish tomorrow.

bendrucker commented 9 years ago

Correct, via thlorenz/proxyquire#65

thlorenz commented 9 years ago

Published as minor upgrade for similar reasons

BTW noticed when running the tests with iojs (which errors for wrong reason) that peer test range still reported all tests as passed. Is that a bug? We need to make sure that failed tests are reported correctly (also on travis).

Here is how to repro (assuming you have n installed):

n io 2.0
npm test

You will see some tests failing but test peer range will report that all passed.

bendrucker commented 9 years ago

Cool. So the issue with the tests is actually not related to peer-range. node test/clientside/run.js && echo $? returns 0 even though there are failures.

https://github.com/substack/tape/blob/25cd3dee215d86a70571fe3faa34e60821abffe8/index.js#L7-L12

Should be a pretty simple fix

thlorenz commented 9 years ago

Ah ok, darn I ran into this so many times already. With tape mostly actually. We're using tap here though. We could try to run our tests one by one and trap the error unless we can get it to work via tap.

Rereading your comment it looks like that won't help if the script itself doesn't return the error code :( Which is weird since we do exit properly here

bendrucker commented 9 years ago

I'll look into this. Part of the issue is that browserify feeds in a process shim and then all the normal exit code management stops, even with vm.runInNewContext. So the options are to pass {detectGlobals: false} and manually pass through the globals or try to parse stdout and exit appropriately. I'll try the former and revert to the latter as a last resort.

bendrucker commented 9 years ago

First rule of programming...

It's a race condition. Even when you pass in process, every bundle gets its own tape. And so the first to exit wins. Sometimes the end result is a failure, sometimes it passes. I think the best approach here is just to pass through tape as a global.