-
Notifications
You must be signed in to change notification settings - Fork 241
Test almost everything #23
Conversation
|
Increased coverage 9% 😎 |
visualization/visualization_test.go
Outdated
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.
may want to check errors here
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.
If os.Getwd() fails then runPerlScript is going to propagate the error when it also calls os.Getwd(). Hence, this test will fail even if I don't check errors here.
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.
sure, i just mean it's bad form to discard errors, and you would want to fail fast here to indicate that something is messed up with your test environment
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.
makes sense
|
looks good |
Added osWrapper struct to visualization.go and main.go to wrap Go os and exec functions. This enables much more test mocking.