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

time: TestLocalZoneAbbr fails when run from Brazil #21183

Closed
jucie opened this issue Jul 26, 2017 · 4 comments

Comments

Projects
None yet
5 participants
@jucie
Copy link

commented Jul 26, 2017

What version of Go are you using (go version)?

go version devel +835dfef Wed Jul 26 13:29:59 2017 +0000 windows/amd64

What operating system and processor architecture are you using (go env)?

set GOARCH=amd64
set GOBIN=
set GOEXE=.exe
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=C:\Users\jucie\go
set GORACE=
set GOROOT=C:\Go
set GOTOOLDIR=C:\Go\pkg\tool\windows_amd64
set GCCGO=gccgo
set CC=gcc
set GOGCCFLAGS=-m64 -mthreads -fmessage-length=0 -fdebug-prefix-map=C:\Users\jucie\AppData\Local\Temp\go-build507317643=/tmp/go-build -gno-record-gcc-switches
set CXX=g++
set CGO_ENABLED=1
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config

What did you do?

Tried to compile and test go toolset.
git pull
cd src
all

What did you expect to see?

A clean compile and test output.

What did you see instead?

--- FAIL: TestLocalZoneAbbr (0.00s)
zoneinfo_windows_test.go:19: Parse failed: parsing time "Wed, 26 Jul 2017 19:01:29 -03" as "Mon, 02 Jan 2006 15:04:05 MST": cannot parse "-03" as "MST"

Obs.: I compiled and tested in Brazil. Brasilia timezone is GMT -03.

session.txt

@jucie jucie changed the title time zoneinfo conversion Error converting time to/from RFC1123 format. Jul 26, 2017

@josharian josharian changed the title Error converting time to/from RFC1123 format. time: TestLocalZoneAbbr fails when run from Brazil Jul 26, 2017

@josharian josharian added this to the Go1.10 milestone Jul 26, 2017

@ALTree

This comment has been minimized.

Copy link
Member

commented Aug 1, 2017

Here's a reproducer that does not require the machine to be in Brasil

package main

import (
	"fmt"
	"time"
)

func main() {
	br, _ := time.LoadLocation("America/Sao_Paulo")
	t1 := time.Now()
	t1 = time.Date(t1.Year(), t1.Month(), t1.Day(),
		t1.Hour(), t1.Minute(), t1.Second(), 0, br)
	_, err := time.Parse(time.RFC1123, t1.Format(time.RFC1123))
	if err != nil {
		fmt.Println(err)
	}
}
$ go run prova.go
parsing time "Tue, 01 Aug 2017 17:35:45 -03" as "Mon, 02 Jan 2006 15:04:05 MST": cannot parse "-03" as "MST"

The issue is in the time.Parse(time.RFC1123, t1.Format(time.RFC1123)) line.

The test assumes that Parse, when called with layout time.RFC1123, will always be able to parse a value generated by Format(time.RFC1123), but this is wrong because even though RFC1123 uses the zone abbreviation (like MST), Format will fallback to the numeric abbreviation (like -07) if the current timezone has no abbreviation, but Parse is stricter and it won't accept -07 in the layout says MST when parsing back the time value.

IANA has been phasing out invented timezone abbreviations, and in the above snippet Format prints "America/Sao_Paulo" as -03, which is rejected by Parse when Parse is given a layout with the abbreviation MST.

A possible fix is to change the test to use time.RFC1123Z, which is like time.RFC1123 except the timezone is always in numeric form (like -03), which is guaranteed to work (unlike invented abbreviations, which as I said IANA is phasing out in newer tzdata releases).

@gopherbot

This comment has been minimized.

Copy link

commented Aug 1, 2017

Change https://golang.org/cl/52430 mentions this issue: time: use RFC1123Z in test to force numeric zone abbreviation

@ALTree

This comment has been minimized.

Copy link
Member

commented Aug 1, 2017

@jucie Any chance you could try out the patch at https://golang.org/cl/52430 and confirm it fixes the issue on your machine?

@ALTree

This comment has been minimized.

Copy link
Member

commented Aug 1, 2017

@jucie Or not, do not try it out. As pointed out it made the test too weak to be actually useful. I'll send a different patch.

@gopherbot gopherbot closed this in 193eda7 Aug 2, 2017

@golang golang locked and limited conversation to collaborators Aug 2, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.