test: deflake job endpoint registration test (#18170)

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.
This commit is contained in:
Tim Gross 2023-08-14 16:17:09 -04:00
parent 04a3628cc4
commit 577d96034d
1 changed files with 9 additions and 27 deletions

View File

@ -25,7 +25,6 @@ import (
"github.com/shoenig/test/must" "github.com/shoenig/test/must"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"golang.org/x/exp/slices"
) )
func TestJobEndpoint_Register(t *testing.T) { func TestJobEndpoint_Register(t *testing.T) {
@ -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) 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 { for _, a := range allocResp.Allocations {
var result int if a.ID == alloc.ID {
// cmp(a, b) should return if cs := a.ClientStatus; cs != structs.AllocClientStatusComplete {
// a positive number when a > b return false, fmt.Errorf("expected old alloc to be complete but found: %s", cs)
if a.CreateIndex > b.CreateIndex { }
result = 1 } else {
if cs := a.ClientStatus; cs != structs.AllocClientStatusPending {
return false, fmt.Errorf("expected new alloc to be pending but found: %s", cs)
}
} }
// a negative number when a < b,
if a.CreateIndex < b.CreateIndex {
result = -1
}
// 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 return true, nil
}) })
} }