Skip to content

Commit

Permalink
DEVPROD-7742 Fix current testifylint lint issues (manual) (evergreen-…
Browse files Browse the repository at this point in the history
  • Loading branch information
ZackarySantana committed Jan 22, 2025
1 parent a010736 commit 385503e
Show file tree
Hide file tree
Showing 15 changed files with 61 additions and 53 deletions.
2 changes: 1 addition & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@ linters:
- govet
- misspell
- unconvert
- unused
- unused
2 changes: 2 additions & 0 deletions model/distro/distro_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,7 @@ func TestGetResolvedPlannerSettings(t *testing.T) {
// Fallback to the SchedulerConfig.ExpectedRuntimeFactor as PlannerSettings.ExpectedRunTimeFactor is equal to 0.
assert.EqualValues(t, 7, resolved0.ExpectedRuntimeFactor)
assert.EqualValues(t, 20, resolved0.GenerateTaskFactor)
//nolint:testifylint // We expect it to be exactly 10.
assert.EqualValues(t, 10, resolved0.NumDependentsFactor)
assert.EqualValues(t, 40, resolved0.StepbackTaskFactor)

Expand Down Expand Up @@ -475,6 +476,7 @@ func TestGetResolvedPlannerSettings(t *testing.T) {
assert.EqualValues(t, 0, resolved1.MainlineTimeInQueueFactor)
assert.EqualValues(t, 0, resolved1.ExpectedRuntimeFactor)
assert.EqualValues(t, 0, resolved1.GenerateTaskFactor)
//nolint:testifylint // We expect it to be exactly 0.
assert.EqualValues(t, 0, resolved1.NumDependentsFactor)

ps := &PlannerSettings{
Expand Down
3 changes: 3 additions & 0 deletions model/reliability/query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,7 @@ func TestGetTaskStatsOneDocument(t *testing.T) {
docs, err := GetTaskReliabilityScores(filter)
require.NoError(err)
require.Len(docs, 1)
//nolint:testifylint // We expect it to be exactly 0.42.
assert.Equal(docs[0].SuccessRate, float64(.42))
}

Expand All @@ -344,7 +345,9 @@ func TestGetTaskStatsTwoDocuments(t *testing.T) {
docs, err := GetTaskReliabilityScores(filter)
require.NoError(err)
require.Len(docs, 2)
//nolint:testifylint // We expect it to be exactly equal.
assert.Equal(docs[0].SuccessRate, float64(.56))
//nolint:testifylint // We expect it to be exactly equal.
assert.Equal(docs[1].SuccessRate, float64(.42))
}

Expand Down
1 change: 1 addition & 0 deletions model/task/expected_duration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ func TestExpectedDuration(t *testing.T) {

results, err := getExpectedDurationsForWindow("", project, bv, now.Add(-1*time.Hour), now)
assert.NoError(err)
//nolint:testifylint // We expect it to be exactly equal.
assert.EqualValues(25*time.Minute, results[0].ExpectedDuration)
assert.InDelta(9.35*float64(time.Minute), results[0].StdDev, 0.01*float64(time.Minute))
}
3 changes: 3 additions & 0 deletions model/task/task_annotations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ func TestAddIssueToAnnotation(t *testing.T) {
assert.NotNil(t, annotation.Issues[0].Source)
assert.Equal(t, annotations.UIRequester, annotation.Issues[0].Source.Requester)
assert.Equal(t, "annie.black", annotation.Issues[0].Source.Author)
//nolint:testifylint // We expect it to be exactly equal.
assert.Equal(t, float64(91.23), annotation.Issues[0].ConfidenceScore)

assert.NoError(t, AddIssueToAnnotation(ctx, "t1", 0, issue, "not.annie.black"))
Expand All @@ -38,6 +39,7 @@ func TestAddIssueToAnnotation(t *testing.T) {
assert.NotNil(t, annotation)
assert.Len(t, annotation.Issues, 2)
assert.NotNil(t, annotation.Issues[1].Source)
//nolint:testifylint // We expect it to be exactly equal.
assert.Equal(t, float64(91.23), annotation.Issues[0].ConfidenceScore)
assert.Equal(t, "not.annie.black", annotation.Issues[1].Source.Author)
dbTask, err := FindOneId("t1")
Expand Down Expand Up @@ -181,6 +183,7 @@ func TestPatchIssue(t *testing.T) {
assert.Equal(t, annotations.UIRequester, annotation.Issues[0].Source.Requester)
assert.Equal(t, "bynn.lee", annotation.Issues[0].Source.Author)
assert.Equal(t, "EVG-1234", annotation.Issues[0].IssueKey)
//nolint:testifylint // We expect it to be exactly equal.
assert.Equal(t, float64(91.23), annotation.Issues[0].ConfidenceScore)
assert.Len(t, annotation.SuspectedIssues, 1)
assert.NotNil(t, annotation.SuspectedIssues[0].Source)
Expand Down
58 changes: 21 additions & 37 deletions model/task_queue_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -973,15 +973,7 @@ func (s *taskDAGDispatchServiceSuite) TestIsRefreshFindNextTaskThreadSafe() {
}
s.Require().NoError(d.Insert(s.ctx))

items := []TaskQueueItem{}
for i := 0; i < 50; i++ {
items = append(items, TaskQueueItem{
Id: fmt.Sprintf("%d", i),
BuildVariant: "variant_1",
Version: "version_1",
Project: "project_1",
GroupMaxHosts: 0,
})
t := task.Task{
Id: fmt.Sprintf("%d", i),
BuildVariant: "variant_1",
Expand Down Expand Up @@ -1029,15 +1021,7 @@ func (s *taskDAGDispatchServiceSuite) TestIsRefreshFindNextTaskThreadSafe() {

func (s *taskDAGDispatchServiceSuite) TestFindNextTaskThreadSafe() {
s.Require().NoError(db.ClearCollections(task.Collection))
items := []TaskQueueItem{}
for i := 0; i < 100; i++ {
items = append(items, TaskQueueItem{
Id: fmt.Sprintf("%d", i),
BuildVariant: "variant_1",
Version: "version_1",
Project: "project_1",
GroupMaxHosts: 0,
})
t := task.Task{
Id: fmt.Sprintf("%d", i),
BuildVariant: "variant_1",
Expand Down Expand Up @@ -1068,7 +1052,13 @@ func (s *taskDAGDispatchServiceSuite) TestFindNextTaskThreadSafe() {
defer wg.Done()
<-wait
item := service.FindNextTask(s.ctx, spec, utility.ZeroTime)
s.Require().NotNil(item)
s.NotNil(item)
if item == nil {
// We cannot require item to not be nil because the check would happen in
// a goroutine. This could cause the test to behave nondeterministically.
// See https://github.com/Antonboom/testifylint?tab=readme-ov-file#go-require.
return
}
mu.Lock()
dispatchedTasks[item.Id] = true
mu.Unlock()
Expand All @@ -1089,18 +1079,9 @@ func (s *taskDAGDispatchServiceSuite) TestFindNextTaskThreadSafe() {

func (s *taskDAGDispatchServiceSuite) TestFindNextTaskGroupTaskThreadSafe() {
s.Require().NoError(db.ClearCollections(task.Collection))
items := []TaskQueueItem{}
for i := 0; i < 20; i++ {
groupNum := i / 5
id := fmt.Sprintf("%d", i)
items = append(items, TaskQueueItem{
Id: id,
Group: fmt.Sprintf("group_%d", groupNum),
BuildVariant: "variant_1",
Version: "version_1",
Project: "project_1",
GroupMaxHosts: 1,
})
t := task.Task{
Id: id,
TaskGroup: fmt.Sprintf("group_%d", groupNum),
Expand Down Expand Up @@ -1137,7 +1118,13 @@ func (s *taskDAGDispatchServiceSuite) TestFindNextTaskGroupTaskThreadSafe() {
defer wg.Done()
<-wait
item := service.FindNextTask(s.ctx, spec, utility.ZeroTime)
s.Require().NotNil(item)
s.NotNil(item)
if item == nil {
// We cannot require item to not be nil because the check would happen in
// a goroutine. This could cause the test to behave nondeterministically.
// See https://github.com/Antonboom/testifylint?tab=readme-ov-file#go-require.
return
}
mu.Lock()
dispatchedTasks[item.Id] = true
mu.Unlock()
Expand All @@ -1156,18 +1143,9 @@ func (s *taskDAGDispatchServiceSuite) TestFindNextTaskGroupTaskThreadSafe() {
s.Equal(dispatchedCount, numGoroutines)

s.Require().NoError(db.ClearCollections(task.Collection))
items = []TaskQueueItem{}
for i := 0; i < 20; i++ {
groupNum := i / 5
id := fmt.Sprintf("%d", i)
items = append(items, TaskQueueItem{
Id: id,
Group: fmt.Sprintf("group_%d", groupNum),
BuildVariant: "variant_1",
Version: "version_1",
Project: "project_1",
GroupMaxHosts: 1,
})
t := task.Task{
Id: id,
TaskGroup: fmt.Sprintf("group_%d", groupNum),
Expand Down Expand Up @@ -1199,7 +1177,13 @@ func (s *taskDAGDispatchServiceSuite) TestFindNextTaskGroupTaskThreadSafe() {
defer wg.Done()
<-wait
item := service.FindNextTask(s.ctx, spec, utility.ZeroTime)
s.Require().NotNil(item)
s.NotNil(item)
if item == nil {
// We cannot require item to not be nil because the check would happen in
// a goroutine. This could cause the test to behave nondeterministically.
// See https://github.com/Antonboom/testifylint?tab=readme-ov-file#go-require.
return
}
mu.Lock()
dispatchedTasks[item.Id] = true
mu.Unlock()
Expand Down
1 change: 1 addition & 0 deletions model/taskstats/query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,7 @@ func (s *statsQuerySuite) checkTaskStats(stats TaskStats, task, variant, distro
require.Equal(numTestFailed, stats.NumTestFailed)
require.Equal(numSystemFailed, stats.NumSystemFailed)
require.Equal(numSetupFailed, stats.NumSetupFailed)
//nolint:testifylint // We expect these to be exactly equal.
require.Equal(avgDuration, stats.AvgDurationSuccess)
}

Expand Down
1 change: 1 addition & 0 deletions model/taskstats/stats_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ func (s *statsSuite) TestGenerateStats() {
s.Equal(2, doc.NumSystemFailed)
s.Equal(3, doc.NumSetupFailed)
s.Equal(2, doc.NumTimeout)
//nolint:testifylint // We expect it to be exactly 150.0.
s.Equal(float64(150), doc.AvgDurationSuccess)
s.WithinDuration(time.Now(), doc.LastUpdate, time.Minute)

Expand Down
4 changes: 2 additions & 2 deletions rest/data/host_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ func (s *HostConnectorSuite) TestSpawnHost() {
s.Contains(err.Error(), "not been allowed by admins")
},
} {
s.T().Run(tName, func(t *testing.T) {
s.Run(tName, func() {
s.Require().NoError(db.ClearCollections(host.Collection))

options := &restmodel.HostRequestOptions{
Expand All @@ -253,7 +253,7 @@ func (s *HostConnectorSuite) TestSpawnHost() {
InstanceTags: nil,
}

tCase(t, options)
tCase(s.T(), options)
})
}
}
Expand Down
27 changes: 15 additions & 12 deletions rest/model/reliability_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,18 +35,21 @@ func TestAPITaskReliabilityBuildFromService(t *testing.T) {

assert.Equal(serviceDoc.TaskName, *apiDoc.TaskName)
assert.Equal(serviceDoc.BuildVariant, *apiDoc.BuildVariant)
assert.Equal(serviceDoc.Distro, *apiDoc.Distro)
assert.Equal(serviceDoc.Date.Format("2006-01-02"), *apiDoc.Date)
assert.Equal(serviceDoc.NumSuccess, apiDoc.NumSuccess)
assert.Equal(serviceDoc.NumFailed, apiDoc.NumFailed)
assert.Equal(serviceDoc.NumTotal, apiDoc.NumTotal)
assert.Equal(serviceDoc.NumTimeout, apiDoc.NumTimeout)
assert.Equal(serviceDoc.NumTestFailed, apiDoc.NumTestFailed)
assert.Equal(serviceDoc.NumSystemFailed, apiDoc.NumSystemFailed)
assert.Equal(serviceDoc.NumSetupFailed, apiDoc.NumSetupFailed)
assert.Equal(serviceDoc.SuccessRate, apiDoc.SuccessRate)
assert.Equal(serviceDoc.AvgDurationSuccess, apiDoc.AvgDurationSuccess)
assert.Equal(serviceDoc.SuccessRate, 8.0)
assert.Equal(*apiDoc.Distro, serviceDoc.Distro)
assert.Equal(*apiDoc.Date, serviceDoc.Date.Format("2006-01-02"))
assert.Equal(apiDoc.NumSuccess, serviceDoc.NumSuccess)
assert.Equal(apiDoc.NumFailed, serviceDoc.NumFailed)
assert.Equal(apiDoc.NumTotal, serviceDoc.NumTotal)
assert.Equal(apiDoc.NumTimeout, serviceDoc.NumTimeout)
assert.Equal(apiDoc.NumTestFailed, serviceDoc.NumTestFailed)
assert.Equal(apiDoc.NumSystemFailed, serviceDoc.NumSystemFailed)
assert.Equal(apiDoc.NumSetupFailed, serviceDoc.NumSetupFailed)
//nolint:testifylint // We expect it to be exactly equal.
assert.Equal(apiDoc.SuccessRate, serviceDoc.SuccessRate)
//nolint:testifylint // We expect it to be exactly equal.
assert.Equal(apiDoc.AvgDurationSuccess, serviceDoc.AvgDurationSuccess)
//nolint:testifylint // We expect it to be exactly 0.0.
assert.Equal(8.0, serviceDoc.SuccessRate)
}

func TestAPITaskReliabilityStartAtKey(t *testing.T) {
Expand Down
1 change: 1 addition & 0 deletions rest/model/task_stats_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ func TestAPITaskStatsBuildFromService(t *testing.T) {
assert.Equal(serviceDoc.NumTestFailed, apiDoc.NumTestFailed)
assert.Equal(serviceDoc.NumSystemFailed, apiDoc.NumSystemFailed)
assert.Equal(serviceDoc.NumSetupFailed, apiDoc.NumSetupFailed)
//nolint:testifylint // We expect it to be exactly equal.
assert.Equal(serviceDoc.AvgDurationSuccess, apiDoc.AvgDurationSuccess)
}

Expand Down
5 changes: 5 additions & 0 deletions rest/route/annotations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,7 @@ func TestAnnotationByTaskGetHandlerRun(t *testing.T) {
assert.Equal(t, "task-1", utility.FromStringPtr(apiAnnotations[0].TaskId))
assert.Equal(t, "task-1-note_1", utility.FromStringPtr(apiAnnotations[0].Note.Message))
require.Len(t, apiAnnotations[0].Issues, 1)
//nolint:testifylint // We expect it to be exactly equal.
assert.Equal(t, float64(12.34), utility.FromFloat64Ptr(apiAnnotations[0].Issues[0].ConfidenceScore))

// get the latest execution : 0
Expand Down Expand Up @@ -682,7 +683,9 @@ func TestAnnotationByTaskPutHandlerRun(t *testing.T) {
assert.Equal(t, "api", annotation.Note.Source.Requester)
assert.Equal(t, "api", annotation.Issues[0].Source.Requester)
assert.Len(t, annotation.Issues, 2)
//nolint:testifylint // We expect it to be exactly equal.
assert.Equal(t, float64(12.34), annotation.Issues[0].ConfidenceScore)
//nolint:testifylint // We expect it to be exactly equal.
assert.Equal(t, float64(56.78), annotation.Issues[1].ConfidenceScore)

//test update
Expand Down Expand Up @@ -715,7 +718,9 @@ func TestAnnotationByTaskPutHandlerRun(t *testing.T) {
require.Nil(t, annotation.SuspectedIssues)
assert.Equal(t, "some key 0", annotation.Issues[0].IssueKey)
assert.Len(t, annotation.Issues, 2)
//nolint:testifylint // We expect it to be exactly equal.
assert.Equal(t, float64(87.65), annotation.Issues[0].ConfidenceScore)
//nolint:testifylint // We expect it to be exactly equal.
assert.Equal(t, float64(43.21), annotation.Issues[1].ConfidenceScore)

//test that it can update old executions
Expand Down
2 changes: 2 additions & 0 deletions rest/route/distro_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -710,6 +710,7 @@ func (s *DistroPatchByIDSuite) TestRunProviderSettingsList() {
s.True(ok)
s.Equal("/dev/xvdb", mappedDoc.Lookup("device_name").StringValue())
s.Equal("ephemeral0", mappedDoc.Lookup("virtual_name").StringValue())
//nolint:testifylint // We expect it to be exactly 0.15.
s.Equal(doc.Lookup("bid_price").Double(), 0.15)
s.Equal("m3.large", doc.Lookup("instance_type").StringValue())
s.Equal("mci", doc.Lookup("key_name").StringValue())
Expand Down Expand Up @@ -1232,6 +1233,7 @@ func (s *DistroPatchByIDSuite) TestValidFindAndReplaceFullDocument() {
s.Equal("~/dev/xvdb", mountPoint.Lookup("device_name").StringValue())
s.Equal("~ephemeral0", mountPoint.Lookup("virtual_name").StringValue())
s.Equal("~ami-2814683f", doc.Lookup("ami").StringValue())
//nolint:testifylint // We expect it to be exactly 0.10.
s.Equal(doc.Lookup("bid_price").Double(), 0.10)
s.Equal("~m3.large", doc.Lookup("instance_type").StringValue())

Expand Down
2 changes: 2 additions & 0 deletions rest/route/reliability_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,7 @@ func TestReliabilityParse(t *testing.T) {
require.NoError(t, err)
require.Equal(t, values["tasks"], handler.filter.Tasks)
require.Equal(t, taskstats.SortLatestFirst, handler.filter.Sort)
//nolint:testifylint // We expect the float to be exactly equal.
require.Equal(t, handler.filter.Significance, reliability.DefaultSignificance)

require.Equal(t, []string{"gitter_request"}, handler.filter.Requesters)
Expand Down Expand Up @@ -430,6 +431,7 @@ func TestReliabilityParse(t *testing.T) {
require.Equal(t, reliability.GroupByDistro, handler.filter.GroupBy) // default value
require.Equal(t, reliability.SortLatestFirst, handler.filter.Sort) // default value
require.Equal(t, reliability.MaxQueryLimit, handler.filter.Limit) // default value
//nolint:testifylint // We expect the float to be exactly 0.1.
require.Equal(t, 0.1, handler.filter.Significance)
},
"Some Values": func(ctx context.Context, t *testing.T, handler taskReliabilityHandler) {
Expand Down
2 changes: 1 addition & 1 deletion units/host_drawdown_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ func TestHostDrawdown(t *testing.T) {
NewCapTarget: 0,
}
num, hosts := numHostsDecommissionedForDrawdown(ctx, t, env, drawdownInfo)
assert.Zero(t, 0, num, "should not draw down host that was recently running task group")
assert.Zero(t, num, "should not draw down host that was recently running task group")
assert.Empty(t, hosts)
},
"DecommissionsIdleMultiHostTaskGroupHost": func(ctx context.Context, t *testing.T, env *mock.Environment, d distro.Distro) {
Expand Down

0 comments on commit 385503e

Please sign in to comment.