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

Simplified tests #47

Merged
merged 1 commit into from
Dec 14, 2014
Merged

Simplified tests #47

merged 1 commit into from
Dec 14, 2014

Conversation

JamesCohen-awin
Copy link
Contributor

I tidied up some of the tests:

  • The tests for mean and percentiles don't rely on the (already tested) parseMessage() function
  • Instead of using time.Now().Unix() a constant is used instead to remove variables from the test and to make simpler (string == string) assertions
  • Modified some test function names to reflect the names of the functions they test

 - using known UNIX timestamps to remove regexps and make assertions clearer
 - removed extra testing/dependency on parseMessage() in mean/% tests
@JamesCohen-awin
Copy link
Contributor Author

also of note for whoever reviews this...

I removed the test from statsdaemon_test.go#L286 as the test string shouldn't appear in the output and wasn't being - even though the test was passing(!)

@mreiferson
Copy link
Contributor

curious what go test -cover reports before/after this change? would you mind pasting?

@JamesCohen-awin
Copy link
Contributor Author

coverage reduces a tiny bit

$ diff /tmp/cover-master.txt /tmp/cover-tidyup.txt 
13c13
< github.com/bitly/statsdaemon/statsdaemon.go:226:  processTimers   100.0%
---
> github.com/bitly/statsdaemon/statsdaemon.go:226:  processTimers   97.4%
17c17
< total:                            (statements)    56.2%
---
> total:                            (statements)    55.6%

It's the "continue" here that is no longer covered:

func processTimers(buffer *bytes.Buffer, now int64, pctls Percentiles) int64 {
        var num int64
        for u, t := range timers {
                if len(t) == 0 {
                        continue
                }

@mreiferson
Copy link
Contributor

k, thanks again 😁

mreiferson added a commit that referenced this pull request Dec 14, 2014
@mreiferson mreiferson merged commit 1ac36f2 into bitly:master Dec 14, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants