-
Notifications
You must be signed in to change notification settings - Fork 17
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
More automated tests #152
More automated tests #152
Conversation
…ds the 100% test coverage goal
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.
lgtm
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.
basename.test.js should cover all branches
busboy-emit.test.js should check if the first call to busboy.emit('finish') returns anything
dicer-write.test.js does not test what it claims
@Uzlopak am I reading the code wrong, or |
test/dicer-write.test.js
Outdated
}) | ||
}) | ||
|
||
t.end() |
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.
Basically not testing anything, because you call t.end(). use t.plan for this subtest also.
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.
@Uzlopak oh, good to know! First time using this testing framework!
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.
remove the t.end() please
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.
@Uzlopak is there anything else for me to do in this PR?
Added more automated tests, chipping in for the 100% test coverage goal mentioned in here
I really don't know the project, if not for what I studied while writing tests, so I picked the least complicated route and wrote a couple of automated tests of "well exposed" functions and methods