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

Improve tests for ExtractGraphX.scala #294

Closed
ruebot opened this Issue Nov 29, 2018 · 3 comments

Comments

Projects
None yet
2 participants
@ruebot
Copy link
Member

ruebot commented Nov 29, 2018

ExtractGraphX.scala test coverage can be improved.

@jrwiebe

This comment has been minimized.

Copy link
Contributor

jrwiebe commented Jan 22, 2019

Is this issue about improving our code coverage score, or are there tests of functionality that you think should be included in ExtractGraphXTest.scala?

If this is about bringing up the code coverage score on ExtractGraphX.scala to 100%, I would contend that isn't a useful target. The codecov report says it's just the case class declarations that aren't being adequately tested. I think this is because case classes contain a number of generated methods, such as equals, copy, and toString, which we aren't testing.

To demonstrate, I added a test of EdgeData's equals method. See how the cobertura report improves. Without equals test:
image
With:
image
Adding a test of toString bumps up the branch coverage by one. Et cetera.

I think as long as we understand there's a valid reason why we are not hitting 100%, it's reasonable, and sensible, not to bother pursuing it.

@ruebot

This comment has been minimized.

Copy link
Member

ruebot commented Jan 22, 2019

If this is about bringing up the code coverage score on ExtractGraphX.scala to 100%, I would contend that isn't a useful target.

Agree!

Background; I went through every file, and just created an issue for it just to documented and be consistent with working one our test coverage. When we started this process, I believe we were around 50% coverage. So, we've improved a fair bit.This particular one isn't that big of a deal. I personally don't mind if we get 100% on it. If you want to add something in for the classes, I'm cool with it. If you don't we can just leave this issue open or close it and tag it won't fix.

I think as long as we understand there's a valid reason why we are not hitting 100%, it's reasonable, and sensible, not to bother pursuing it.

Agree!

@jrwiebe

This comment has been minimized.

Copy link
Contributor

jrwiebe commented Jan 22, 2019

Let's close and tag it won't-fix.

@jrwiebe jrwiebe closed this Jan 22, 2019

@jrwiebe jrwiebe added the wontfix label Jan 22, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment