-
Notifications
You must be signed in to change notification settings - Fork 52
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
Working travis config + simple integration test + node 4 support #8
Conversation
Since it's a test that is supposed to fail to demonstrate leak testing.
Avoiding require() destructuring as well.
Btw: I think publishing a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once the tests are updated to use iterate
, I think this will be good to go! (Or just update one of the two, that way both the deprecated and new names are being tested.)
And yes, I think a v0.2.0
is in order! Once it's out, I'll update the PR in my repository to use the newer version and test it out.
it('throws an error when testing leaky code', () => { | ||
const objects = [] | ||
|
||
expect(() => leakage.iteration(100, () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3945066 later in this PR merges the change that renames iteration
-> iterate
, but this test is never updated to the new name, so it's only the deprecated export that's being tested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And that, children, is why you should never skip code reviews... 😅
Fixes #3 and #4.
@btmills