From 577d96034d4bc8fbc25b67b04cb50984055616c1 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Mon, 14 Aug 2023 16:17:09 -0400 Subject: [PATCH] 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. --- nomad/job_endpoint_test.go | 36 +++++++++--------------------------- 1 file changed, 9 insertions(+), 27 deletions(-) diff --git a/nomad/job_endpoint_test.go b/nomad/job_endpoint_test.go index 79ba20506..87bbe3ef6 100644 --- a/nomad/job_endpoint_test.go +++ b/nomad/job_endpoint_test.go @@ -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) { @@ -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 + 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) + } } - // 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 }) }