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

Covered UUID.decodePlain() with wrong input with a test #24

Merged
merged 1 commit into from
Jul 18, 2018

Conversation

zerkms
Copy link
Member

@zerkms zerkms commented Jul 17, 2018

The TestDecodePlainWithWrongLength runs unexported method since there is no other way to reach that line.

It's a substitution for the #22 which I accidentally closed via deleting my branch (instead of hard-push'ing it)

@coveralls
Copy link

coveralls commented Jul 17, 2018

Pull Request Test Coverage Report for Build 77

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.6%) to 99.063%

Totals Coverage Status
Change from base Build 76: 0.6%
Covered Lines: 317
Relevant Lines: 320

💛 - Coveralls

@theckman
Copy link
Member

@zerkms I just destroyed your branch, I'm sorry. :(

@zerkms
Copy link
Member Author

zerkms commented Jul 17, 2018

It's fine, I need to include a tiny change there anyway :-)

@zerkms zerkms force-pushed the CODEC_AND_SQL_CODE_COVERAGE branch from 91b8200 to 36ec376 Compare July 17, 2018 08:28
@zerkms zerkms changed the title Added couple tests that increase code test coverage Covered UUID.decodePlain() with wrong input with a test Jul 17, 2018
Copy link
Member

@acln0 acln0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Copy link
Member

@niaow niaow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

codec_test.go Outdated
arg := []byte{'4', '2'}

u := UUID{}
err := u.decodePlain(arg)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zerkms I might be nitpicking here a bit. Since we do not use the error value, how do we feel about simplifying this to the single if expression?

if u.decodePlain(arg) == nil {}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The alternative I'd go with, if we care about the error type, would be:

var err error
// ...
if err = u.decodePlain(arg); err == nil {}

Just so that we assert the return type as being an error value.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't mind in either, but would personally prefer the first (given every other test case does not care about the exact returned err type either).

@zerkms zerkms force-pushed the CODEC_AND_SQL_CODE_COVERAGE branch from 36ec376 to 774487f Compare July 18, 2018 06:48
@zerkms zerkms merged commit 1ce602f into gofrs:master Jul 18, 2018
@zerkms zerkms deleted the CODEC_AND_SQL_CODE_COVERAGE branch July 18, 2018 06:57
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

Successfully merging this pull request may close these issues.

5 participants