Skip to content
This repository was archived by the owner on Sep 11, 2020. It is now read-only.

Work around a Go bug when parsing timezones #320

Merged
merged 1 commit into from
Mar 31, 2017
Merged

Work around a Go bug when parsing timezones #320

merged 1 commit into from
Mar 31, 2017

Conversation

lupine
Copy link

@lupine lupine commented Mar 30, 2017

On my machine, which has a local time of +0100, commits with a timezone of +0000 adopt the local timezone. This is noted as a bug in golang/go#19750 - assigned to the Go 1.9 milestone.

This workaround fixes commit timezone parsing in the interim.

I don't know a way to reproduce this error in a test case, so there are no spec changes, but I've manually tested the change and it does resolve the issue for me locally. Here's the same test case, before and after:

=== RUN   TestIndexingGitlabTest
2017/03/30 19:32:18 Indexing from 0000000000000000000000000000000000000000 to b83d6e391c22777fca1ed3012fce84f633d7fed0
2017/03/30 19:32:18 Index: gitlab-test-1490898738, Project ID: 667
--- FAIL: TestIndexingGitlabTest (0.75s)
        Error Trace:    integration_test.go:163
        Error:          Not equal:
                        expected: map[string]interface {}{"type":"commit", "sha":"b83d6e391c22777fca1ed3012fce84f633d7fed0", "author":map[string]interface {}{"email":"[email protected]", "name":"Job van der Voort", "time":"20160927T143746+0000"}, "committer":map[string]interface {}{"name":"Job van der Voort", "time":"20160927T143746+0000", "email":"[email protected]"}, "rid":"667", "message":"Merge branch 'branch-merged' into 'master'\r\n\r\nadds bar folder and branch-test text file to check Repository merged_to_root_ref method\r\n\r\n\r\n\r\nSee merge request !12"}
                        received: map[string]interface {}{"rid":"667", "message":"Merge branch 'branch-merged' into 'master'\r\n\r\nadds bar folder and branch-test text file to check Repository merged_to_root_ref method\r\n\r\n\r\n\r\nSee merge request !12", "sha":"b83d6e391c22777fca1ed3012fce84f633d7fed0", "type":"commit", "author":map[string]interface {}{"name":"Job van der Voort", "email":"[email protected]", "time":"20160927T153746+0100"}, "committer":map[string]interface {}{"name":"Job van der Voort", "email":"[email protected]", "time":"20160927T153746+0100"}}

                        Diff:
                        --- Expected
                        +++ Actual
                        @@ -4,3 +4,3 @@
                           (string) (len=4) "name": (string) (len=17) "Job van der Voort",
                        -  (string) (len=4) "time": (string) (len=20) "20160927T143746+0000"
                        +  (string) (len=4) "time": (string) (len=20) "20160927T153746+0100"
                          },
                        @@ -9,3 +9,3 @@
                           (string) (len=4) "name": (string) (len=17) "Job van der Voort",
                        -  (string) (len=4) "time": (string) (len=20) "20160927T143746+0000"
                        +  (string) (len=4) "time": (string) (len=20) "20160927T153746+0100"
                          },
FAIL
=== RUN   TestIndexingGitlabTest
2017/03/30 19:37:34 Indexing from 0000000000000000000000000000000000000000 to b83d6e391c22777fca1ed3012fce84f633d7fed0
2017/03/30 19:37:34 Index: gitlab-test-1490899054, Project ID: 667
--- PASS: TestIndexingGitlabTest (0.78s)
PASS

Closes #318

@lupine
Copy link
Author

lupine commented Mar 30, 2017

@mcuadros how's this?

@codecov
Copy link

codecov bot commented Mar 30, 2017

Codecov Report

Merging #320 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #320   +/-   ##
=======================================
  Coverage   77.35%   77.35%           
=======================================
  Files         117      117           
  Lines        8015     8015           
=======================================
  Hits         6200     6200           
  Misses       1156     1156           
  Partials      659      659
Impacted Files Coverage Δ
plumbing/object/object.go 67.39% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e512b02...bdb72de. Read the comment docs.

@mcuadros
Copy link
Contributor

Please provide a test, since is a very arcane issue

@lupine
Copy link
Author

lupine commented Mar 30, 2017

@mcuadros my only reproducer involves changing the system time at the moment. I can keep hunting, but since this involves unexported behaviour in time, there's no guarantee that I can make something reasonable.

@mcuadros mcuadros merged commit 31a249d into src-d:master Mar 31, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Golang bug breaks timezone parsing in Signature.Decode
2 participants