Skip to content

Commit

Permalink
Fixes tests with correct assumptions
Browse files Browse the repository at this point in the history
  • Loading branch information
cyriltovena committed Dec 7, 2023
1 parent 570ed7a commit 850dd66
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 8 deletions.
9 changes: 5 additions & 4 deletions pkg/api/version/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ var (
instanceTimeout = 1 * time.Minute
_ memberlist.Mergeable = (*Versions)(nil)
_ services.Service = (*Service)(nil)
now = time.Now
)

func GetCodec() codec.Codec {
Expand Down Expand Up @@ -63,7 +64,7 @@ func (v *Versions) Marshal() ([]byte, error) {

// Merge merges two versions. This is used when CASing or merging versions from other nodes.
// v is the local version and should be mutated to include the changes from incoming.
// The returned value is the change to broadcast.
// The function should only returned changed instances.
func (v *Versions) Merge(incoming memberlist.Mergeable, localCAS bool) (memberlist.Mergeable, error) {
if incoming == nil {
return nil, nil
Expand Down Expand Up @@ -107,7 +108,7 @@ func (v *Versions) Merge(incoming memberlist.Mergeable, localCAS bool) (memberli
for k, current := range v.Instances {
if _, ok := other.Instances[k]; !ok && !current.Left {
current.Left = true
current.Timestamp = time.Now().UnixNano()
current.Timestamp = now().UnixNano()
updated = append(updated, k)
}
}
Expand Down Expand Up @@ -138,7 +139,7 @@ func (d *Versions) MergeContent() []string {
func (v *Versions) RemoveTombstones(limit time.Time) (total, removed int) {
for n, inst := range v.Instances {
if inst.Left {
if limit.IsZero() || time.Unix(inst.Timestamp, 0).Before(limit) {
if limit.IsZero() || time.Unix(0, inst.Timestamp).Before(limit) {
// remove it
delete(v.Instances, n)
removed++
Expand Down Expand Up @@ -246,7 +247,7 @@ func (svc *Service) heartbeat(ctx context.Context) error {
}
current.Addr = svc.addr
current.ID = svc.id
current.Timestamp = time.Now().UnixNano()
current.Timestamp = now().UnixNano()
current.QuerierAPI = svc.version
// Now prune old instances.
for id, instance := range versions.Instances {
Expand Down
24 changes: 20 additions & 4 deletions pkg/api/version/version_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,16 @@ func TestVersionsMultiple(t *testing.T) {
expectVersion(t, 2)
}

var nowTs = time.Now().UnixNano()

func TestMerge(t *testing.T) {
now = func() time.Time {
return time.Unix(0, nowTs)
}
t.Cleanup(func() {
now = time.Now
})

for name, tc := range map[string]struct {
base *Versions
incoming memberlist.Mergeable
Expand Down Expand Up @@ -200,7 +209,6 @@ func TestMerge(t *testing.T) {
createVersion(t, "2", 3),
),
expected: createVersions(t,
createVersion(t, "1", 1),
createVersion(t, "2", 3),
),
},
Expand All @@ -213,7 +221,6 @@ func TestMerge(t *testing.T) {
createVersion(t, "2", 2),
),
expected: createVersions(t,
createVersion(t, "1", 1),
createVersion(t, "2", 2),
),
},
Expand All @@ -226,7 +233,7 @@ func TestMerge(t *testing.T) {
createVersion(t, "1", 1),
),
expected: createVersions(t,
createVersion(t, "1", 1),
createLeftVersion(t, "2", nowTs),
),
},
"instance removed and added": {
Expand All @@ -240,7 +247,7 @@ func TestMerge(t *testing.T) {
),
expected: createVersions(t,
createVersion(t, "3", 3),
createVersion(t, "2", 2),
createLeftVersion(t, "1", nowTs),
),
},
} {
Expand All @@ -254,6 +261,15 @@ func TestMerge(t *testing.T) {
}
}

func createLeftVersion(t *testing.T, id string, ts int64) *versionv1.InstanceVersion {
t.Helper()
return &versionv1.InstanceVersion{
ID: id,
Timestamp: ts,
Left: true,
}
}

func createVersion(t *testing.T, id string, ts int64) *versionv1.InstanceVersion {
t.Helper()
return &versionv1.InstanceVersion{
Expand Down

0 comments on commit 850dd66

Please sign in to comment.