Skip to content

Commit

Permalink
test(datastore): replace non-UTC time test
Browse files Browse the repository at this point in the history
This test caused race conditions by manipulating time.Local and
there is not a good way to test the behavior while avoiding this.
Instead I replace it by passing in timezone explicitly in
setVal and writing a test for that.

Fixes googleapis#3402
  • Loading branch information
tritone committed Dec 15, 2020
1 parent 87150a8 commit 440d44f
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 40 deletions.
12 changes: 9 additions & 3 deletions datastore/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,12 @@ func plsFieldLoad(v reflect.Value, p Property, subfields []string) (ok bool, err

// setVal sets 'v' to the value of the Property 'p'.
func setVal(v reflect.Value, p Property) (s string) {
return setValInTimezone(v, p, time.UTC)
}

// setVal sets 'v' to the value of the Property 'p', using
// timezone 'loc'.
func setValInTimezone(v reflect.Value, p Property, loc *time.Location) (s string) {
pValue := p.Value
switch v.Kind() {
case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
Expand Down Expand Up @@ -384,13 +390,13 @@ func setVal(v reflect.Value, p Property) (s string) {
}
v.Set(reflect.ValueOf(x))
case typeOfCivilDate:
date := civil.DateOf(pValue.(time.Time).In(time.UTC))
date := civil.DateOf(pValue.(time.Time).In(loc))
v.Set(reflect.ValueOf(date))
case typeOfCivilDateTime:
dateTime := civil.DateTimeOf(pValue.(time.Time).In(time.UTC))
dateTime := civil.DateTimeOf(pValue.(time.Time).In(loc))
v.Set(reflect.ValueOf(dateTime))
case typeOfCivilTime:
timeVal := civil.TimeOf(pValue.(time.Time).In(time.UTC))
timeVal := civil.TimeOf(pValue.(time.Time).In(loc))
v.Set(reflect.ValueOf(timeVal))
default:
ent, ok := pValue.(*Entity)
Expand Down
52 changes: 15 additions & 37 deletions datastore/load_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package datastore

import (
"fmt"
"log"
"reflect"
"testing"
"time"
Expand Down Expand Up @@ -439,43 +440,6 @@ type withUntypedInterface struct {
Field interface{}
}

func TestLoadCivilTimeInNonUTCZone(t *testing.T) {
t.Skip("https://github.com/googleapis/google-cloud-go/issues/3402")
src := &pb.Entity{
Key: keyToProto(testKey0),
Properties: map[string]*pb.Value{
"Time": {ValueType: &pb.Value_TimestampValue{TimestampValue: &timestamppb.Timestamp{Seconds: 1605504600}}},
},
}
dst := &struct{ Time civil.Time }{
Time: civil.Time{},
}
want := &struct{ Time civil.Time }{
Time: civil.Time{
Hour: 5,
Minute: 30,
},
}
loc, err := time.LoadLocation("Africa/Cairo")
if err != nil {
t.Fatalf("LoadLocation: %v", err)
}
time.Local = loc

err = loadEntityProto(dst, src)
if err != nil {
t.Fatalf("loadEntityProto: %v", err)
}
if diff := testutil.Diff(dst, want); diff != "" {
t.Fatalf("Mismatch: got - want +\n%s", diff)
}
loc, err = time.LoadLocation("UTC")
if err != nil {
t.Fatalf("LoadLocation: %v", err)
}
time.Local = loc
}

func TestLoadToInterface(t *testing.T) {
testCases := []struct {
name string
Expand Down Expand Up @@ -606,6 +570,20 @@ func TestLoadToInterface(t *testing.T) {
}
}

func TestCivilNonUTCTimezone(t *testing.T) {
p := Property{
Name: "civilTimeProp",
Value: time.Time{},
}
ct := civil.Time{}
var v reflect.Value
v = reflect.New(reflect.TypeOf(ct))
log.Printf("v type: %v\n", reflect.TypeOf(ct))

s := setValInTimezone(v, p, time.UTC)
log.Printf("returned: %v", s)
}

func TestAlreadyPopulatedDst(t *testing.T) {
testCases := []struct {
desc string
Expand Down

0 comments on commit 440d44f

Please sign in to comment.