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

Implement duration parser #8

Merged
merged 3 commits into from
Dec 10, 2017
Merged

Implement duration parser #8

merged 3 commits into from
Dec 10, 2017

Conversation

mxpv
Copy link
Contributor

@mxpv mxpv commented Aug 5, 2017

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

@eduncan911
Copy link
Owner

eduncan911 commented Aug 7, 2017

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
Copy link
Contributor Author

mxpv commented Aug 9, 2017

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

Copy link
Owner

@eduncan911 eduncan911 left a comment

Choose a reason for hiding this comment

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

@mxpv yeah, I think goveralls fails on remote branches (ran into the same issue before). As long the tests pass I'm fine.

Btw, code coverage has dropped from 100% to 99.2%. Try running:

go test -coverprofile=coverage.out 
go tool cover -html=coverage.out

And see what lines are missing when viewing in the browser that opens.

I'm out of town at the moment so I'll look myself when I get back.

@eduncan911 eduncan911 self-assigned this Aug 20, 2017
@mxpv
Copy link
Contributor Author

mxpv commented Aug 23, 2017

@eduncan911 done, should be 100% now

@eduncan911
Copy link
Owner

Thanks again!

@eduncan911 eduncan911 merged commit 10213c6 into eduncan911:master Dec 10, 2017
eduncan911 pushed a commit that referenced this pull request Feb 4, 2020
* Correct count len of UTF8 strings (#9)

* Correctly count len of UTF8 strings (fix panics)
* Sort header
* Fixes Panics

* Will implement duration parser (#8)

* Implement duration parser
* Fix example
* Increase coverage to 100%

* Fix Github and GoDocs Markdown (#14)

* GH Markdown fixes

* Updated format for godoc

* Move podcast.go Private Methods to Respected Files (#12)

* close #10

* remove exported function types

* remove parseDateRFC1123Z function type

* Allow providing GUID on Podcast (#15)

* Allow providing own GUID

* Check length instead of comparing to empty string

* Adding test for user supplied GUID

* Remove coveralls token

Co-authored-by: Maksym Pavlenko <makpav@amazon.com>
Co-authored-by: Konstantin Chukhlomin <mail@chuhlomin.com>
Co-authored-by: iwittkau <iwittkau@users.noreply.github.com>
Co-authored-by: Damian Szeluga <damian.szeluga@gmail.com>
@eduncan911 eduncan911 mentioned this pull request Feb 4, 2020
eduncan911 added a commit that referenced this pull request Feb 5, 2020
* Catching Develop up to Master (#17)

* Correct count len of UTF8 strings (#9)

* Correctly count len of UTF8 strings (fix panics)
* Sort header
* Fixes Panics

* Will implement duration parser (#8)

* Implement duration parser
* Fix example
* Increase coverage to 100%

* Fix Github and GoDocs Markdown (#14)

* GH Markdown fixes

* Updated format for godoc

* Move podcast.go Private Methods to Respected Files (#12)

* close #10

* remove exported function types

* remove parseDateRFC1123Z function type

* Allow providing GUID on Podcast (#15)

* Allow providing own GUID

* Check length instead of comparing to empty string

* Adding test for user supplied GUID

* Remove coveralls token

Co-authored-by: Maksym Pavlenko <makpav@amazon.com>
Co-authored-by: Konstantin Chukhlomin <mail@chuhlomin.com>
Co-authored-by: iwittkau <iwittkau@users.noreply.github.com>
Co-authored-by: Damian Szeluga <damian.szeluga@gmail.com>

* Update doc.go for release notes

* Update README for Release

Co-authored-by: Jader Brasil <jaderebrasil@gmail.com>
Co-authored-by: Maksym Pavlenko <makpav@amazon.com>
Co-authored-by: Konstantin Chukhlomin <mail@chuhlomin.com>
Co-authored-by: iwittkau <iwittkau@users.noreply.github.com>
Co-authored-by: Damian Szeluga <damian.szeluga@gmail.com>
eduncan911 added a commit that referenced this pull request Feb 7, 2020
# This is the 1st commit message:

Go Mod and Vendor

# This is the commit message #2:

asd

# This is the commit message #3:

asd

# This is the commit message #4:

asd

# This is the commit message #5:

asd

# This is the commit message #6:

asd

# This is the commit message #7:

asd

# This is the commit message #8:

asd

# This is the commit message #9:

asd
bestony referenced this pull request in bestony/podcast Jun 4, 2020
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.

2 participants