search

fluffle / sp0rkle

sp0rkle is dead, long live sp0rkle
irc://irc.pl0rt.org/#ed
8 stars 4 forks source link

Date format has changes #65

Closed gundalow closed 8 years ago

gundalow commented 8 years ago

9623531d840d7635f9925030fef0c96fc9e8734b changed the format of the timestrings. This change should make the unit tests pass on local machines (but not Travis)

The following now passes on UK systems:

time go test -v  ./...

Review on Reviewable

gundalow commented 8 years ago

Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion.

_drivers/factdriver/plugin_test.go, line 34 [r1] (raw file):_ This looks strange as it's $time $date

But I believe it's correct.

Comments from the review on Reviewable.io

fluffle commented 8 years ago

Review status: 0 of 1 files reviewed at latest revision, 2 unresolved discussions.

_drivers/factdriver/plugin_test.go, line 19 [r1] (raw file):_ As noted on IRC you need to do flag.Set("timezone", "UTC") here.

_drivers/factdriver/plugin_test.go, line 34 [r1] (raw file):_ ... and then this will need s/GMT/UTC/, but then tests should pass everywhere.

Comments from the review on Reviewable.io

fluffle commented 8 years ago

I deliberately removed those things from .gitignore, so please ditch that commit from this pull request and delete those files.

fluffle commented 8 years ago

It looks like you need to fix up datetime_test.go too, sorry. Two things.

1) Add flag.Set() as before in TestParseTimeFormats() 2) s/time.Local/time.UTC/ on line 28.

My bad, but it's nice that Travis runs tests in a different timezone and catches this kind of fail :-)

Review status: 0 of 2 files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

Comments from the review on Reviewable.io

fluffle commented 8 years ago

Reviewed 1 of 1 files at r2, 1 of 2 files at r3, 1 of 1 files at r5. Review status: all files reviewed at latest revision, all discussions resolved.

Comments from the review on Reviewable.io