Skip to content

Commit

Permalink
fix: sort slices before comparing in test (#142)
Browse files Browse the repository at this point in the history
## Description

The Runner test
`TestRunner_GetBaseActionCfg/extraEnv_adds_and_overrides_defaults` in
[`src/pkg/runner/actions_test.go`](https://github.com/defenseunicorns/maru-runner/blob/535258256cd1714f55e643075ae15919316e4aec/src/pkg/runner/actions_test.go#L502)
is flaky because we don't sort the string slices before comparing them,
and occasionally they get created in slightly different orders:

```zsh
=== RUN   TestRunner_GetBaseActionCfg/extraEnv_adds_and_overrides_defaults
    actions_test.go:502:
                Error Trace:    /Users/clint/go/github.com/defenseunicorns/maru-runner/src/pkg/runner/actions_test.go:502
                Error:          Not equal:
                                expected: []string{"ENV1=fromDefault", "ENV2=xyz1", "ENV4=fromSet", "ENV2=alsoFromEnv", "ENV3=fromExtra"}
                                actual  : []string{"ENV1=fromDefault", "ENV2=xyz1", "ENV4=fromSet", "ENV3=fromExtra", "ENV2=alsoFromEnv"}
```

note the expected `"ENV2=alsoFromEnv", "ENV3=fromExtra"` vs. the actual
`"ENV3=fromExtra", "ENV2=alsoFromEnv"`.

Assuming a successful test run produces this output:

```zsh
=== RUN   TestRunner_GetBaseActionCfg
=== RUN   TestRunner_GetBaseActionCfg/extraEnv_adds_and_overrides_defaults
--- PASS: TestRunner_GetBaseActionCfg (0.00s)
    --- PASS: TestRunner_GetBaseActionCfg/extraEnv_adds_and_overrides_defaults (0.00s)
```

If we run the test enough times it will fail, as shown below where we
would expect 200 lines of `-- PASS` here when running the test 100 times
(2x per run), and 0 for `--- FAIL`:

```zsh
 github.com/defenseunicorns/maru-runner on  main [$]
 ➜ go test ./src/pkg/runner/... -v -run=TestRunner_GetBaseActionCfg/extraEnv_adds_and_overrides_defaults -count=100 | grep '\-\-\- PASS' | wc -l
     142

 github.com/defenseunicorns/maru-runner on  main
 ➜ go test ./src/pkg/runner/... -v -run=TestRunner_GetBaseActionCfg/extraEnv_adds_and_overrides_defaults -count=100 | grep '\-\-\- FAIL' | wc -l
      46
```

It's flaky, so your numbers will vary each time you run it. 

In this PR we sort both slices before comparing. 
Here's the updated output:

```zsh
 github.com/defenseunicorns/maru-runner on  fix-runner-test-sort
 ➜ go test ./src/pkg/runner/... -v -run=TestRunner_GetBaseActionCfg/extraEnv_adds_and_overrides_defaults -count=100 | grep '\-\-\- PASS' | wc -l
     200

 github.com/defenseunicorns/maru-runner on  fix-runner-test-sort
 ➜ go test ./src/pkg/runner/... -v -run=TestRunner_GetBaseActionCfg/extraEnv_adds_and_overrides_defaults -count=100 | grep '\-\-\- FAIL' | wc -l
       0
```

## Type of change

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Other (security config, docs update, etc)

## Checklist before merging

- [x] Test, docs, adr added or updated as needed
- [x] [Contributor Guide
Steps](https://github.com/defenseunicorns/maru-runner/blob/main/CONTRIBUTING.md)
followed

Signed-off-by: catsby <clint@ctshryock.com>
  • Loading branch information
catsby authored Sep 18, 2024
1 parent 5352582 commit 77f52c9
Showing 1 changed file with 3 additions and 1 deletion.
4 changes: 3 additions & 1 deletion src/pkg/runner/actions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package runner

import (
"reflect"
"slices"
"testing"

"github.com/defenseunicorns/maru-runner/src/config"
Expand Down Expand Up @@ -498,7 +499,8 @@ func TestRunner_GetBaseActionCfg(t *testing.T) {
}

got := GetBaseActionCfg(tt.args.cfg, tt.args.a, tt.args.vars)

slices.Sort(got.Env)
slices.Sort(tt.want)
require.Equal(t, tt.want, got.Env, "The returned Env array did not match what was wanted")
})
}
Expand Down

0 comments on commit 77f52c9

Please sign in to comment.