Skip to content

Commit

Permalink
Deflake the v1 exit code tests
Browse files Browse the repository at this point in the history
It appears that there is a timing related bug hiding in the original
implementation. In this commit, we borrow the v2 version of the test
after making some changes to dry it up a bit.
  • Loading branch information
angrycub committed Oct 26, 2023
1 parent 013d853 commit 02c955d
Show file tree
Hide file tree
Showing 2 changed files with 125 additions and 124 deletions.
121 changes: 60 additions & 61 deletions internal/cli/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,15 @@ import (
// TODO: Test outputPlannedJob that returns non-zero exit code

const (
testPack = "simple_raw_exec"
testRef = "48eb7d5"
testRefFlag = "--ref=" + testRef
badACLToken = "bad00000-bad0-bad0-bad0-badbadbadbad"
testPack = "simple_raw_exec"
testRef = "48eb7d5"
testRefFlag = "--ref=" + testRef
badACLToken = "bad00000-bad0-bad0-bad0-badbadbadbad"
exitcodeMakesChanges = 91
exitcodeNoChanges = 90
exitcodeError = 92

testPlanCmdString = "plan --exit-code-no-changes=90 --exit-code-makes-changes=91 --exit-code-error=92"
)

func TestCLI_CreateTestRegistry(t *testing.T) {
Expand Down Expand Up @@ -194,85 +199,79 @@ func TestCLI_JobPlan_ConflictingNonPackJob(t *testing.T) {

func TestCLI_PackPlan_OverrideExitCodes(t *testing.T) {
ct.HTTPTest(t, ct.WithDefaultConfig(), func(s *agent.TestAgent) {
testPlanCommand := func(t *testing.T) []string {
out := strings.Split(testPlanCmdString, " ")
out = append(out, getTestPackPath(t, testPack))
return out
}

t.Run("plan_against_empty", func(t *testing.T) {
// Plan against empty - should be makes-changes
result := runTestPackCmd(t, s, []string{
"plan",
"--exit-code-makes-changes=91",
"--exit-code-no-changes=90",
"--exit-code-error=92",
getTestPackPath(t, testPack),
})
result := runTestPackCmd(t, s, testPlanCommand(t))
must.Eq(t, "", result.cmdErr.String(), must.Sprintf("cmdErr should be empty, but was %q", result.cmdErr.String()))
must.StrContains(t, result.cmdOut.String(), "Plan succeeded\n")
must.Eq(t, 91, result.exitCode) // Should return exit-code-makes-changes
must.Eq(t, exitcodeMakesChanges, result.exitCode) // Should return exit-code-makes-changes
})

t.Run("register non-pack-job", func(t *testing.T) {
// Register non pack job
err := ct.NomadRun(s, getTestNomadJobPath(t, testPack))
must.NoError(t, err)
})
// Register non pack job
err := ct.NomadRun(s, getTestNomadJobPath(t, testPack))
must.NoError(t, err)

t.Run("register_pack_expect_error", func(t *testing.T) {
// Now try to register the pack, should make error
result := runTestPackCmd(t, s, []string{
"plan",
"--exit-code-makes-changes=91",
"--exit-code-no-changes=90",
"--exit-code-error=92",
getTestPackPath(t, testPack),
})
// Now try to register the pack, should be error
result := runTestPackCmd(t, s, testPlanCommand(t))
must.Eq(t, "", result.cmdErr.String(), must.Sprintf("cmdErr should be empty, but was %q", result.cmdErr.String()))
must.StrContains(t, result.cmdOut.String(), job.ErrExistsNonPack{JobID: testPack}.Error())
must.Eq(t, 92, result.exitCode) // Should exit-code-error
must.Eq(t, exitcodeError, result.exitCode) // Should exit-code-error
})

err = ct.NomadPurge(s, testPack)
must.NoError(t, err)
t.Run("cleanup non-pack-job", func(t *testing.T) {
// Purge the non-pack job created earlier.
err := ct.NomadPurge(s, testPack)
must.NoError(t, err)

isGone := func() bool {
_, err = ct.NomadJobStatus(s, testPack)
if err != nil {
return err.Error() == "Unexpected response code: 404 (job not found)"
isGone := func() bool {
_, err = ct.NomadJobStatus(s, testPack)
if err != nil {
return err.Error() == "Unexpected response code: 404 (job not found)"
}
return false
}
return false
}
must.Wait(t, wait.InitialSuccess(
wait.BoolFunc(isGone),
wait.Timeout(10*time.Second),
wait.Gap(500*time.Millisecond),
), must.Sprint("test job failed to purge"))
must.Wait(t, wait.InitialSuccess(
wait.BoolFunc(isGone),
wait.Timeout(10*time.Second),
wait.Gap(500*time.Millisecond),
), must.Sprint("test job failed to purge"))
})

// Make a pack deployment so we can validate the "no-change" condition
result := runTestPackCmd(t, s, []string{"run", getTestPackPath(t, testPack)})
must.Eq(t, "", result.cmdErr.String(), must.Sprintf("cmdErr should be empty, but was %q", result.cmdErr.String()))
must.StrContains(t, result.cmdOut.String(), "")
must.Zero(t, result.exitCode) // Should return 0
isStarted := func() bool {
j, err := ct.NomadJobStatus(s, testPack)
if err != nil {
return false
t.Run("setup for pack_against_deployed", func(t *testing.T) {
result := runTestPackCmd(t, s, []string{"run", getTestPackPath(t, testPack)})
must.Eq(t, "", result.cmdErr.String(), must.Sprintf("cmdErr should be empty, but was %q", result.cmdErr.String()))
must.StrContains(t, result.cmdOut.String(), "")
must.Zero(t, result.exitCode) // Should return 0
isStarted := func() bool {
j, err := ct.NomadJobStatus(s, testPack)
if err != nil {
return false
}
return *j.Status == "running"
}
return *j.Status == "running"
}
must.Wait(t, wait.InitialSuccess(
wait.BoolFunc(isStarted),
wait.Timeout(30*time.Second),
wait.Gap(500*time.Millisecond),
), must.Sprint("test job failed to start"))
must.Wait(t, wait.InitialSuccess(
wait.BoolFunc(isStarted),
wait.Timeout(30*time.Second),
wait.Gap(500*time.Millisecond),
), must.Sprint("test job failed to start"))
})

t.Run("pack_against_deployed", func(t *testing.T) {

// Plan against deployed - should be no-changes
result = runTestPackCmd(t, s, []string{
"plan",
"--exit-code-makes-changes=91",
"--exit-code-no-changes=90",
"--exit-code-error=92",
getTestPackPath(t, testPack),
})
result := runTestPackCmd(t, s, testPlanCommand(t))
must.Eq(t, "", result.cmdErr.String(), must.Sprintf("cmdErr should be empty, but was %q", result.cmdErr.String()))
must.StrContains(t, result.cmdOut.String(), "Plan succeeded\n")
must.Eq(t, 90, result.exitCode, must.Sprintf("stdout:\n%s\n\nstderr:\n%s\n", result.cmdOut.String(), result.cmdErr.String())) // Should return exit-code-no-changes
must.Eq(t, exitcodeNoChanges, result.exitCode, must.Sprintf("stdout:\n%s\n\nstderr:\n%s\n", result.cmdOut.String(), result.cmdErr.String())) // Should return exit-code-no-changes
})
})
}
Expand Down
128 changes: 65 additions & 63 deletions internal/cli/cli_v1_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,79 +143,81 @@ func TestCLI_V1_JobPlan_ConflictingNonPackJob(t *testing.T) {

func TestCLI_V1_PackPlan_OverrideExitCodes(t *testing.T) {
ct.HTTPTest(t, ct.WithDefaultConfig(), func(s *agent.TestAgent) {
// Plan against empty - should be makes-changes
result := runTestPackV1Cmd(t, s, []string{
"plan",
"--exit-code-makes-changes=91",
"--exit-code-no-changes=90",
"--exit-code-error=92",
getTestPackV1Path(t, testPack),
testPlanCommand := func(t *testing.T) []string {
out := strings.Split(testPlanCmdString, " ")
out = append(out, getTestPackV1Path(t, testPack))
return out
}
t.Run("plan_against_empty", func(t *testing.T) {
// Plan against empty - should be makes-changes
result := runTestPackV1Cmd(t, s, testPlanCommand(t))
must.Eq(t, "", result.cmdErr.String(), must.Sprintf("cmdErr should be empty, but was %q", result.cmdErr.String()))
must.StrContains(t, result.cmdOut.String(), "Plan succeeded\n")
must.Eq(t, exitcodeMakesChanges, result.exitCode) // Should return exit-code-makes-changes
})
must.Eq(t, 91, result.exitCode) // Should return exit-code-makes-changes
expectNoStdErrOutput(t, result)
must.StrContains(t, result.cmdOut.String(), "Plan succeeded\n")

// Register non pack job
err := ct.NomadRun(s, getTestNomadJobPath(t, testPack))
must.NoError(t, err)
t.Run("register non-pack-job", func(t *testing.T) {
// Register non pack job
err := ct.NomadRun(s, getTestNomadJobPath(t, testPack))
must.NoError(t, err)
})

// Now try to register the pack, should make error
result = runTestPackV1Cmd(t, s, []string{
"plan",
"--exit-code-makes-changes=91",
"--exit-code-no-changes=90",
"--exit-code-error=92",
getTestPackV1Path(t, testPack),
t.Run("register_pack_expect_error", func(t *testing.T) {
// Now try to register the pack, should make error
result := runTestPackV1Cmd(t, s, testPlanCommand(t))
must.Eq(t, "", result.cmdErr.String(), must.Sprintf("cmdErr should be empty, but was %q", result.cmdErr.String()))
must.StrContains(t, result.cmdOut.String(), job.ErrExistsNonPack{JobID: testPack}.Error())
must.Eq(t, exitcodeError, result.exitCode) // Should exit-code-error
})
must.Eq(t, 92, result.exitCode) // Should exit-code-error
expectNoStdErrOutput(t, result)
must.StrContains(t, result.cmdOut.String(), job.ErrExistsNonPack{JobID: testPack}.Error())

err = ct.NomadPurge(s, testPack)
must.NoError(t, err)
t.Run("cleanup non-pack-job", func(t *testing.T) {
err := ct.NomadPurge(s, testPack)
must.NoError(t, err)

isGone := func() bool {
_, err = ct.NomadJobStatus(s, testPack)
if err != nil {
return err.Error() == "Unexpected response code: 404 (job not found)"
isGone := func() bool {
_, err = ct.NomadJobStatus(s, testPack)
if err != nil {
return err.Error() == "Unexpected response code: 404 (job not found)"
}
return false
}
return false
}
must.Wait(t, wait.InitialSuccess(
wait.BoolFunc(isGone),
wait.Timeout(10*time.Second),
wait.Gap(500*time.Millisecond),
), must.Sprint("test job failed to purge"))

result = runTestPackV1Cmd(t, s, []string{"run", getTestPackV1Path(t, testPack)})
must.Zero(t, result.exitCode) // Should return 0
expectNoStdErrOutput(t, result)
must.StrContains(t, result.cmdOut.String(), "")
must.Wait(t, wait.InitialSuccess(
wait.BoolFunc(isGone),
wait.Timeout(10*time.Second),
wait.Gap(500*time.Millisecond),
), must.Sprint("test job failed to purge"))
})

isStarted := func() bool {
j, err := ct.NomadJobStatus(s, testPack)
if err != nil {
return false
// Make a pack deployment so we can validate the "no-change" condition
t.Run("setup for pack_against_deployed", func(t *testing.T) {
// scoping this part because I reuse result in the last part
result := runTestPackV1Cmd(t, s, []string{"run", getTestPackV1Path(t, testPack)})
must.Eq(t, "", result.cmdErr.String(), must.Sprintf("cmdErr should be empty, but was %q", result.cmdErr.String()))
must.StrContains(t, result.cmdOut.String(), "")
must.Zero(t, result.exitCode) // Should return 0
isStarted := func() bool {
j, err := ct.NomadJobStatus(s, testPack)
if err != nil {
return false
}
return *j.Status == "running"
}
return *j.Status == "running"
}
must.Wait(t, wait.InitialSuccess(
wait.BoolFunc(isStarted),
wait.Timeout(30*time.Second),
wait.Gap(500*time.Millisecond),
), must.Sprint("test job failed to start"))

// Plan against deployed - should be no-changes
result = runTestPackV1Cmd(t, s, []string{
"plan",
"--exit-code-makes-changes=91",
"--exit-code-no-changes=90",
"--exit-code-error=92",
getTestPackV1Path(t, testPack),
must.Wait(t, wait.InitialSuccess(
wait.BoolFunc(isStarted),
wait.Timeout(30*time.Second),
wait.Gap(500*time.Millisecond),
), must.Sprint("test job failed to start"))
})

t.Run("pack_against_deployed", func(t *testing.T) {

// Plan against deployed - should be no-changes
result := runTestPackV1Cmd(t, s, testPlanCommand(t))

must.Eq(t, "", result.cmdErr.String(), must.Sprintf("cmdErr should be empty, but was %q", result.cmdErr.String()))
must.StrContains(t, result.cmdOut.String(), "Plan succeeded\n")
must.Eq(t, exitcodeNoChanges, result.exitCode, must.Sprintf("stdout:\n%s\n\nstderr:\n%s\n", result.cmdOut.String(), result.cmdErr.String())) // Should return exit-code-no-changes
})
must.Eq(t, 90, result.exitCode) // Should return exit-code-no-changes
expectNoStdErrOutput(t, result)
must.StrContains(t, result.cmdOut.String(), "Plan succeeded\n")
})
}

Expand Down

0 comments on commit 02c955d

Please sign in to comment.