eduncan911 / podcast

iTunes and RSS 2.0 Podcast Generator in Golang
MIT License
132 stars 35 forks source link

Implement duration parser #8

Closed mxpv closed 6 years ago

mxpv commented 7 years ago

This implements parseDuration func + test @eduncan911 please take a look on this. Thanks.

eduncan911 commented 7 years ago

I like it.

Though, the tests are failing.

I need to finish my blog post about this pattern I came up with here, as I really like it and use it for all our projects now.

In short, Examples = Valid Tests, they can't be ignored. The Examples cover the good tests cases, and from the external API perspective as well. It provides code coverage, good test cases, and extended documentation of actual working examples - all in a single test!

The internal tests, as you have added to here, usually tests only the bad cases - or tests that need internal states of the package mutated to make them pass or fail.

Otherwise, I try to use Examples to test as much code as possible for the reasons mentioned above. Anything I cannot test with Examples (say, code that cannot be triggered from the external API methods), I switch to using internal tests. But that is done as a last resort.

Examples cover 80% of all test cases in this repo. And look damn doing it too:

http://godoc.org/github.com/eduncan911/podcast#pkg-examples


Could you change the existing Example Tests to make them pass? Just run go test -cover locally and you'll see the output of the Examples. Sometimes they may be hard to diff (Go Example output diff isn't their strong suite). This is where I usually dump them into two files, and use diff to compare.

But I'm pretty sure what is failing is that I was previously output, say, 36063 as the total seconds (as that is compatible with iTunes). But with this change, it would now show in the Output of the example, "10:01:03".

Could you take a look for each Example that is failing, check out the Duration that is being set, and change the expected Output to what it should be?

I'll also take a stab at updating it when I get some spare cycles.

mxpv commented 7 years ago

Hey. I've fixed an example, but it's still failing on goveralls step :(

mxpv commented 7 years ago

@eduncan911 done, should be 100% now

eduncan911 commented 6 years ago

Thanks again!