From a404807dd63231f9850cb560ebf0472438d6a294 Mon Sep 17 00:00:00 2001
From: Noble Mittal <62551163+beingnoble03@users.noreply.github.com>
Date: Tue, 13 Feb 2024 16:47:12 +0530
Subject: [PATCH] Add missing tests for `go/vt/vtorc/collection` (#15070)

Signed-off-by: Noble Mittal <noblemittal@outlook.com>
---
 go/vt/vtorc/collection/collection.go      |   4 +-
 go/vt/vtorc/collection/collection_test.go | 127 ++++++++++++++++++++++
 2 files changed, 129 insertions(+), 2 deletions(-)

diff --git a/go/vt/vtorc/collection/collection.go b/go/vt/vtorc/collection/collection.go
index 0ef9a71b9a3..4f679286978 100644
--- a/go/vt/vtorc/collection/collection.go
+++ b/go/vt/vtorc/collection/collection.go
@@ -260,8 +260,8 @@ func (c *Collection) removeBefore(t time.Time) error {
 	// get the interval we need.
 	if first == len(c.collection) {
 		c.collection = nil // remove all entries
-	} else if first != -1 {
-		c.collection = c.collection[first:]
+	} else {
+		c.collection = c.collection[first+1:]
 	}
 	return nil // no errors
 }
diff --git a/go/vt/vtorc/collection/collection_test.go b/go/vt/vtorc/collection/collection_test.go
index 23679245c26..16ea6943dc7 100644
--- a/go/vt/vtorc/collection/collection_test.go
+++ b/go/vt/vtorc/collection/collection_test.go
@@ -19,6 +19,8 @@ package collection
 import (
 	"testing"
 	"time"
+
+	"github.com/stretchr/testify/assert"
 )
 
 var randomString = []string{
@@ -28,6 +30,7 @@ var randomString = []string{
 
 // some random base timestamp
 var ts = time.Date(2016, 12, 27, 13, 36, 40, 0, time.Local)
+var ts2 = ts.AddDate(-1, 0, 0)
 
 // TestCreateOrReturn tests the creation of a named Collection
 func TestCreateOrReturnCollection(t *testing.T) {
@@ -87,6 +90,13 @@ func (tm *testMetric) When() time.Time {
 	return ts
 }
 
+type testMetric2 struct {
+}
+
+func (tm *testMetric2) When() time.Time {
+	return ts2
+}
+
 // check that Append() works as expected
 func TestAppend(t *testing.T) {
 	c := &Collection{}
@@ -101,4 +111,121 @@ func TestAppend(t *testing.T) {
 			t.Errorf("TestExpirePeriod: len(Metrics) = %d, expecting %d", len(c.Metrics()), v)
 		}
 	}
+
+	// Test for nil metric
+	err := c.Append(nil)
+	assert.Error(t, err)
+	assert.Equal(t, err.Error(), "Collection.Append: m == nil")
+}
+
+func TestNilCollection(t *testing.T) {
+	var c *Collection
+
+	metrics := c.Metrics()
+	assert.Nil(t, metrics)
+
+	err := c.Append(nil)
+	assert.Error(t, err)
+	assert.Equal(t, err.Error(), "Collection.Append: c == nil")
+
+	err = c.removeBefore(ts)
+	assert.Error(t, err)
+	assert.Equal(t, err.Error(), "Collection.removeBefore: c == nil")
+
+	// Should not throw any error for nil Collection
+	c.StartAutoExpiration()
+	c.StopAutoExpiration()
+}
+
+func TestStopAutoExpiration(t *testing.T) {
+	oldNamedCollection := namedCollection
+	defer func() {
+		namedCollection = oldNamedCollection
+	}()
+	// Clear Collection map
+	namedCollection = make(map[string]*Collection)
+
+	name := randomString[0]
+	c := CreateOrReturnCollection(name)
+
+	c.StopAutoExpiration()
+	assert.False(t, c.monitoring)
+
+	// Test when c.monitoring == true before calling StartAutoExpiration
+	c.monitoring = true
+	c.StartAutoExpiration()
+	assert.True(t, c.monitoring)
+}
+
+func TestSince(t *testing.T) {
+	oldNamedCollection := namedCollection
+	defer func() {
+		namedCollection = oldNamedCollection
+	}()
+	// Clear Collection map
+	namedCollection = make(map[string]*Collection)
+
+	name := randomString[0]
+
+	var c *Collection
+	metrics, err := c.Since(ts)
+
+	assert.Nil(t, metrics)
+	assert.Error(t, err)
+	assert.Equal(t, err.Error(), "Collection.Since: c == nil")
+
+	c = CreateOrReturnCollection(name)
+	metrics, err = c.Since(ts)
+	assert.Nil(t, metrics)
+	assert.Nil(t, err)
+
+	tm := &testMetric{}
+	tm2 := &testMetric2{}
+	_ = c.Append(tm2)
+	_ = c.Append(tm)
+
+	metrics, err = c.Since(ts2)
+	assert.Equal(t, []Metric{tm2, tm}, metrics)
+	assert.Nil(t, err)
+
+	metrics, err = c.Since(ts)
+	assert.Equal(t, []Metric{tm}, metrics)
+	assert.Nil(t, err)
+}
+
+func TestRemoveBefore(t *testing.T) {
+	oldNamedCollection := namedCollection
+	defer func() {
+		namedCollection = oldNamedCollection
+	}()
+	// Clear Collection map
+	namedCollection = make(map[string]*Collection)
+
+	name := randomString[0]
+	c := CreateOrReturnCollection(name)
+
+	tm := &testMetric{}
+	tm2 := &testMetric2{}
+
+	err := c.Append(tm2)
+	assert.Nil(t, err)
+
+	err = c.Append(tm)
+	assert.Nil(t, err)
+
+	err = c.removeBefore(ts)
+	assert.NoError(t, err)
+	assert.Equal(t, []Metric{tm}, c.collection)
+
+	ts3 := ts.AddDate(1, 0, 0)
+	err = c.removeBefore(ts3)
+	assert.NoError(t, err)
+	assert.Nil(t, c.collection)
+
+	name = randomString[1]
+	c = CreateOrReturnCollection(name)
+
+	err = c.removeBefore(ts)
+	assert.NoError(t, err)
+	assert.Equal(t, []Metric(nil), c.collection)
 }