Skip to content

Commit

Permalink
test: deflake job endpoint registration test (#18170)
Browse files Browse the repository at this point in the history
We've seen test flakiness in the `TestJobEndpoint_Register_NonOverlapping` test,
which asserts that we don't try to placed allocations for blocked evals until
resources have been actually freed by setting the client status of the previous
alloc to complete.

The flaky assertion includes sorting the two allocations by CreateIndex and this
appears to be a non-stable sort in the context of the test run, which results in
failures that shouldn't exist. There's no reason to sort the allocations instead
of just examining them by ID. This changeset does so.
  • Loading branch information
tgross committed Aug 14, 2023
1 parent a88c930 commit e3986a8
Showing 1 changed file with 9 additions and 27 deletions.
36 changes: 9 additions & 27 deletions nomad/job_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"github.com/shoenig/test/must"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"golang.org/x/exp/slices"
)

func TestJobEndpoint_Register(t *testing.T) {
Expand Down Expand Up @@ -267,35 +266,18 @@ func TestJobEndpoint_Register_NonOverlapping(t *testing.T) {
return false, fmt.Errorf("expected 2 allocs but found %d:\n%v", n, allocResp.Allocations)
}

slices.SortFunc(allocResp.Allocations, func(a, b *structs.AllocListStub) int {
var result int
// cmp(a, b) should return
// a positive number when a > b
if a.CreateIndex > b.CreateIndex {
result = 1
}
// a negative number when a < b,
if a.CreateIndex < b.CreateIndex {
result = -1
for _, a := range allocResp.Allocations {
if a.ID == alloc.ID {
if cs := a.ClientStatus; cs != structs.AllocClientStatusComplete {
return false, fmt.Errorf("expected old alloc to be complete but found: %s", cs)
}
} else {
if cs := a.ClientStatus; cs != structs.AllocClientStatusPending {
return false, fmt.Errorf("expected new alloc to be pending but found: %s", cs)
}
}
// zero when a == b.
result = 0

// invert the comparison to sort descending.
return result * -1
})

if alloc.ID != allocResp.Allocations[0].ID {
return false, fmt.Errorf("unexpected change in alloc: %#v", *allocResp.Allocations[0])
}

if cs := allocResp.Allocations[0].ClientStatus; cs != structs.AllocClientStatusComplete {
return false, fmt.Errorf("expected old alloc to be complete but found: %s", cs)
}

if cs := allocResp.Allocations[1].ClientStatus; cs != structs.AllocClientStatusPending {
return false, fmt.Errorf("expected new alloc to be pending but found: %s", cs)
}
return true, nil
})
}
Expand Down

0 comments on commit e3986a8

Please sign in to comment.