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

Pre.js unit test are very broken. #61

Closed
ariporad opened this issue Aug 19, 2015 · 1 comment
Closed

Pre.js unit test are very broken. #61

ariporad opened this issue Aug 19, 2015 · 1 comment

Comments

@ariporad
Copy link
Contributor

As I discovered with lots and lots of debugging while working on #60, the unit tests for pre.js (test/specs/pre.js) are badly broken.

With the old commit-analyzer API, the commit-analyzer would be called, and could trigger a release, even if no commits were present. For this reason, no body realized that pre.js was not actually stubbing child_process, the commit-analyzer was being called anyway.

Well, with the new API, the commit-analyzer is only called when needed, so the mocking has to work. Unfortunately, it doesn't.

I'm not really sure how the tests should be redone, AFAIK, proxyquire sandboxes the module, so the mock won't trickle down into commit.js. So I decided to open this to discuss it.

cc @boennemann

@boennemann
Copy link
Member

Thanks for catching this very nasty bug! I didn't realize nested stubbing isn't possible, but after reading the docs it makes a lot of sense.

I addressed this now by wrapping the proxyquire calls into each other: 5cc7da6#diff-b2fc167aa2bf97d4559ff48a142cf3e4R6

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