From a3cc46043100e3acfa939f8529685c10f9e77ab5 Mon Sep 17 00:00:00 2001 From: David Timm Date: Fri, 12 Apr 2019 11:43:11 -0600 Subject: [PATCH 1/5] Add more nil checks to metav1.Time and MicroTime Co-authored-by: Ben Fuller --- .../pkg/apis/meta/v1/micro_time.go | 31 +++++++++++++------ .../apimachinery/pkg/apis/meta/v1/time.go | 10 +++--- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/micro_time.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/micro_time.go index 6f6c5111bc8..cdd9a6a7a0c 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/micro_time.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/micro_time.go @@ -41,11 +41,6 @@ func (t *MicroTime) DeepCopyInto(out *MicroTime) { *out = *t } -// String returns the representation of the time. -func (t MicroTime) String() string { - return t.Time.String() -} - // NewMicroTime returns a wrapped instance of the provided time func NewMicroTime(time time.Time) MicroTime { return MicroTime{time} @@ -72,22 +67,40 @@ func (t *MicroTime) IsZero() bool { // Before reports whether the time instant t is before u. func (t *MicroTime) Before(u *MicroTime) bool { - return t.Time.Before(u.Time) + if t != nil && u != nil { + return t.Time.Before(u.Time) + } + return false } // Equal reports whether the time instant t is equal to u. func (t *MicroTime) Equal(u *MicroTime) bool { - return t.Time.Equal(u.Time) + if t == nil && u == nil { + return true + } + if t != nil && u != nil { + return t.Time.Equal(u.Time) + } + return false } // BeforeTime reports whether the time instant t is before second-lever precision u. func (t *MicroTime) BeforeTime(u *Time) bool { - return t.Time.Before(u.Time) + if t != nil && u != nil { + return t.Time.Before(u.Time) + } + return false } // EqualTime reports whether the time instant t is equal to second-lever precision u. func (t *MicroTime) EqualTime(u *Time) bool { - return t.Time.Equal(u.Time) + if t == nil && u == nil { + return true + } + if t != nil && u != nil { + return t.Time.Equal(u.Time) + } + return false } // UnixMicro returns the local time corresponding to the given Unix time diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/time.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/time.go index 79904fd7943..fe510ed9e6d 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/time.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/time.go @@ -41,11 +41,6 @@ func (t *Time) DeepCopyInto(out *Time) { *out = *t } -// String returns the representation of the time. -func (t Time) String() string { - return t.Time.String() -} - // NewTime returns a wrapped instance of the provided time func NewTime(time time.Time) Time { return Time{time} @@ -72,7 +67,10 @@ func (t *Time) IsZero() bool { // Before reports whether the time instant t is before u. func (t *Time) Before(u *Time) bool { - return t.Time.Before(u.Time) + if t != nil && u != nil { + return t.Time.Before(u.Time) + } + return false } // Equal reports whether the time instant t is equal to u. From 4f99aafb7df6588c14b7fe14b09eca36f1dd90bc Mon Sep 17 00:00:00 2001 From: Ben Fuller Date: Mon, 15 Apr 2019 14:53:15 -0600 Subject: [PATCH 2/5] Time/MicroTime: add unit tests for comparison functions Co-authored-by: David Timm --- .../pkg/apis/meta/v1/micro_time_test.go | 116 ++++++++++++++++++ .../pkg/apis/meta/v1/time_test.go | 44 +++++++ 2 files changed, 160 insertions(+) diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/micro_time_test.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/micro_time_test.go index b08e3f236c5..fac8f56e76a 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/micro_time_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/micro_time_test.go @@ -137,3 +137,119 @@ func TestMicroTimeProto(t *testing.T) { } } } + +func TestMicroTimeEqual(t *testing.T) { + t1 := NewMicroTime(time.Now()) + cases := []struct { + name string + x *MicroTime + y *MicroTime + result bool + }{ + {"nil =? nil", nil, nil, true}, + {"!nil =? !nil", &t1, &t1, true}, + {"nil =? !nil", nil, &t1, false}, + {"!nil =? nil", &t1, nil, false}, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + result := c.x.Equal(c.y) + if result != c.result { + t.Errorf("Failed equality test for '%v', '%v': expected %+v, got %+v", c.x, c.y, c.result, result) + } + }) + } +} + +func TestMicroTimeEqualTime(t *testing.T) { + t1 := NewMicroTime(time.Now()) + t2 := NewTime(t1.Time) + cases := []struct { + name string + x *MicroTime + y *Time + result bool + }{ + {"nil =? nil", nil, nil, true}, + {"!nil =? !nil", &t1, &t2, true}, + {"nil =? !nil", nil, &t2, false}, + {"!nil =? nil", &t1, nil, false}, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + result := c.x.EqualTime(c.y) + if result != c.result { + t.Errorf("Failed equality test for '%v', '%v': expected %+v, got %+v", c.x, c.y, c.result, result) + } + }) + } +} + +func TestMicroTimeBefore(t *testing.T) { + t1 := NewMicroTime(time.Now()) + cases := []struct { + name string + x *MicroTime + y *MicroTime + }{ + {"nil Date: Mon, 15 Apr 2019 15:59:40 -0600 Subject: [PATCH 3/5] Update gofmt Co-authored-by: David Timm --- .../pkg/apis/meta/v1/micro_time_test.go | 16 ++++++++-------- .../apimachinery/pkg/apis/meta/v1/time_test.go | 8 ++++---- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/micro_time_test.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/micro_time_test.go index fac8f56e76a..3b274578689 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/micro_time_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/micro_time_test.go @@ -190,9 +190,9 @@ func TestMicroTimeEqualTime(t *testing.T) { func TestMicroTimeBefore(t *testing.T) { t1 := NewMicroTime(time.Now()) cases := []struct { - name string - x *MicroTime - y *MicroTime + name string + x *MicroTime + y *MicroTime }{ {"nil Date: Tue, 16 Apr 2019 10:32:28 -0600 Subject: [PATCH 4/5] MicroTime/Time: remove pointer receivers and redundant IsZero() method Co-authored-by: David Timm --- .../pkg/apis/meta/v1/micro_time.go | 42 ++----- .../pkg/apis/meta/v1/micro_time_test.go | 112 ++++++------------ .../apimachinery/pkg/apis/meta/v1/time.go | 25 +--- .../pkg/apis/meta/v1/time_test.go | 64 +++------- ...pis_meta_v1_unstructed_unstructure_test.go | 4 +- 5 files changed, 65 insertions(+), 182 deletions(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/micro_time.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/micro_time.go index cdd9a6a7a0c..b35bf88533d 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/micro_time.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/micro_time.go @@ -57,50 +57,24 @@ func NowMicro() MicroTime { return MicroTime{time.Now()} } -// IsZero returns true if the value is nil or time is zero. -func (t *MicroTime) IsZero() bool { - if t == nil { - return true - } - return t.Time.IsZero() -} - // Before reports whether the time instant t is before u. -func (t *MicroTime) Before(u *MicroTime) bool { - if t != nil && u != nil { - return t.Time.Before(u.Time) - } - return false +func (t MicroTime) Before(u MicroTime) bool { + return t.Time.Before(u.Time) } // Equal reports whether the time instant t is equal to u. -func (t *MicroTime) Equal(u *MicroTime) bool { - if t == nil && u == nil { - return true - } - if t != nil && u != nil { - return t.Time.Equal(u.Time) - } - return false +func (t MicroTime) Equal(u MicroTime) bool { + return t.Time.Equal(u.Time) } // BeforeTime reports whether the time instant t is before second-lever precision u. -func (t *MicroTime) BeforeTime(u *Time) bool { - if t != nil && u != nil { - return t.Time.Before(u.Time) - } - return false +func (t MicroTime) BeforeTime(u Time) bool { + return t.Time.Before(u.Time) } // EqualTime reports whether the time instant t is equal to second-lever precision u. -func (t *MicroTime) EqualTime(u *Time) bool { - if t == nil && u == nil { - return true - } - if t != nil && u != nil { - return t.Time.Equal(u.Time) - } - return false +func (t MicroTime) EqualTime(u Time) bool { + return t.Time.Equal(u.Time) } // UnixMicro returns the local time corresponding to the given Unix time diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/micro_time_test.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/micro_time_test.go index 3b274578689..248f6c32e09 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/micro_time_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/micro_time_test.go @@ -140,115 +140,73 @@ func TestMicroTimeProto(t *testing.T) { func TestMicroTimeEqual(t *testing.T) { t1 := NewMicroTime(time.Now()) - cases := []struct { - name string - x *MicroTime - y *MicroTime - result bool - }{ - {"nil =? nil", nil, nil, true}, - {"!nil =? !nil", &t1, &t1, true}, - {"nil =? !nil", nil, &t1, false}, - {"!nil =? nil", &t1, nil, false}, - } + t2 := NewMicroTime(time.Now().Add(time.Second)) - for _, c := range cases { - t.Run(c.name, func(t *testing.T) { - result := c.x.Equal(c.y) - if result != c.result { - t.Errorf("Failed equality test for '%v', '%v': expected %+v, got %+v", c.x, c.y, c.result, result) - } - }) + if !t1.Equal(t1) { + t.Errorf("Failed equality test for '%v', '%v': t1 should equal t1", t1, t1) + } + if t1.Equal(t2) { + t.Errorf("Failed equality test for '%v', '%v': t1 should not equal t2", t1, t2) } } func TestMicroTimeEqualTime(t *testing.T) { t1 := NewMicroTime(time.Now()) t2 := NewTime(t1.Time) - cases := []struct { - name string - x *MicroTime - y *Time - result bool - }{ - {"nil =? nil", nil, nil, true}, - {"!nil =? !nil", &t1, &t2, true}, - {"nil =? !nil", nil, &t2, false}, - {"!nil =? nil", &t1, nil, false}, - } + t3 := NewTime(time.Now().Add(time.Second)) - for _, c := range cases { - t.Run(c.name, func(t *testing.T) { - result := c.x.EqualTime(c.y) - if result != c.result { - t.Errorf("Failed equality test for '%v', '%v': expected %+v, got %+v", c.x, c.y, c.result, result) - } - }) + if !t1.EqualTime(t2) { + t.Errorf("Failed equality test for '%v', '%v': t1 should equal t2", t1, t1) + } + if t1.EqualTime(t3) { + t.Errorf("Failed equality test for '%v', '%v': t1 should not equal t3", t1, t2) } } func TestMicroTimeBefore(t *testing.T) { - t1 := NewMicroTime(time.Now()) + tBefore := NewMicroTime(time.Now()) + tAfter := NewMicroTime(tBefore.Time.Add(time.Second)) cases := []struct { - name string - x *MicroTime - y *MicroTime + name string + x MicroTime + y MicroTime + result bool }{ - {"nil Date: Tue, 16 Apr 2019 11:03:35 -0600 Subject: [PATCH 5/5] Revert "MicroTime/Time: remove pointer receivers and redundant IsZero() method" This reverts commit e129d701789f3a97312892618a32f0b266d8d98f. Co-authored-by: David Timm --- .../pkg/apis/meta/v1/micro_time.go | 42 +++++-- .../pkg/apis/meta/v1/micro_time_test.go | 114 ++++++++++++------ .../apimachinery/pkg/apis/meta/v1/time.go | 25 +++- .../pkg/apis/meta/v1/time_test.go | 66 +++++++--- ...pis_meta_v1_unstructed_unstructure_test.go | 4 +- 5 files changed, 184 insertions(+), 67 deletions(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/micro_time.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/micro_time.go index b35bf88533d..cdd9a6a7a0c 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/micro_time.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/micro_time.go @@ -57,24 +57,50 @@ func NowMicro() MicroTime { return MicroTime{time.Now()} } +// IsZero returns true if the value is nil or time is zero. +func (t *MicroTime) IsZero() bool { + if t == nil { + return true + } + return t.Time.IsZero() +} + // Before reports whether the time instant t is before u. -func (t MicroTime) Before(u MicroTime) bool { - return t.Time.Before(u.Time) +func (t *MicroTime) Before(u *MicroTime) bool { + if t != nil && u != nil { + return t.Time.Before(u.Time) + } + return false } // Equal reports whether the time instant t is equal to u. -func (t MicroTime) Equal(u MicroTime) bool { - return t.Time.Equal(u.Time) +func (t *MicroTime) Equal(u *MicroTime) bool { + if t == nil && u == nil { + return true + } + if t != nil && u != nil { + return t.Time.Equal(u.Time) + } + return false } // BeforeTime reports whether the time instant t is before second-lever precision u. -func (t MicroTime) BeforeTime(u Time) bool { - return t.Time.Before(u.Time) +func (t *MicroTime) BeforeTime(u *Time) bool { + if t != nil && u != nil { + return t.Time.Before(u.Time) + } + return false } // EqualTime reports whether the time instant t is equal to second-lever precision u. -func (t MicroTime) EqualTime(u Time) bool { - return t.Time.Equal(u.Time) +func (t *MicroTime) EqualTime(u *Time) bool { + if t == nil && u == nil { + return true + } + if t != nil && u != nil { + return t.Time.Equal(u.Time) + } + return false } // UnixMicro returns the local time corresponding to the given Unix time diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/micro_time_test.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/micro_time_test.go index 248f6c32e09..3b274578689 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/micro_time_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/micro_time_test.go @@ -140,73 +140,115 @@ func TestMicroTimeProto(t *testing.T) { func TestMicroTimeEqual(t *testing.T) { t1 := NewMicroTime(time.Now()) - t2 := NewMicroTime(time.Now().Add(time.Second)) - - if !t1.Equal(t1) { - t.Errorf("Failed equality test for '%v', '%v': t1 should equal t1", t1, t1) + cases := []struct { + name string + x *MicroTime + y *MicroTime + result bool + }{ + {"nil =? nil", nil, nil, true}, + {"!nil =? !nil", &t1, &t1, true}, + {"nil =? !nil", nil, &t1, false}, + {"!nil =? nil", &t1, nil, false}, } - if t1.Equal(t2) { - t.Errorf("Failed equality test for '%v', '%v': t1 should not equal t2", t1, t2) + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + result := c.x.Equal(c.y) + if result != c.result { + t.Errorf("Failed equality test for '%v', '%v': expected %+v, got %+v", c.x, c.y, c.result, result) + } + }) } } func TestMicroTimeEqualTime(t *testing.T) { t1 := NewMicroTime(time.Now()) t2 := NewTime(t1.Time) - t3 := NewTime(time.Now().Add(time.Second)) - - if !t1.EqualTime(t2) { - t.Errorf("Failed equality test for '%v', '%v': t1 should equal t2", t1, t1) + cases := []struct { + name string + x *MicroTime + y *Time + result bool + }{ + {"nil =? nil", nil, nil, true}, + {"!nil =? !nil", &t1, &t2, true}, + {"nil =? !nil", nil, &t2, false}, + {"!nil =? nil", &t1, nil, false}, } - if t1.EqualTime(t3) { - t.Errorf("Failed equality test for '%v', '%v': t1 should not equal t3", t1, t2) + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + result := c.x.EqualTime(c.y) + if result != c.result { + t.Errorf("Failed equality test for '%v', '%v': expected %+v, got %+v", c.x, c.y, c.result, result) + } + }) } } func TestMicroTimeBefore(t *testing.T) { - tBefore := NewMicroTime(time.Now()) - tAfter := NewMicroTime(tBefore.Time.Add(time.Second)) + t1 := NewMicroTime(time.Now()) cases := []struct { - name string - x MicroTime - y MicroTime - result bool + name string + x *MicroTime + y *MicroTime }{ - {"tBefore