appliance: fix test flakiness when container env vars are specified#62732
Conversation
Since introducing the standard feature for container env vars in https://github.com/sourcegraph/sourcegraph/pull/62624, some test flakiness has been introduced. This is because container.go was iterating through an unsorted map, which made golden file assertions non-deterministic. This is fixed by sorting the map keys when appending user-provided env vars.
| for k, v := range ctrConfig.EnvVars { | ||
| ctr.Env = append(ctr.Env, corev1.EnvVar{Name: k, Value: v}) | ||
| } | ||
| ctr.Env = append(ctr.Env, newSortedEnvVars(ctrConfig.EnvVars)...) |
There was a problem hiding this comment.
I thought about replacing this with something that generically returns sorted map entries from the maps package, but erred on the side of quick-and-simple here. We'd have to write a generic sort.Interface implementation to accomplish that, and since we're only ever operating on string keys in this package, sort.Strings() was sitting right there...
There was a problem hiding this comment.
I ran into something similar to this in the POC as well but handled it in the resource level: https://github.com/sourcegraph/sourcegraph-operator/blob/main/internal/controller/resource/container/container.go#L87
jdpleiness
left a comment
There was a problem hiding this comment.
Looks good, just comments 😄
| for k, v := range ctrConfig.EnvVars { | ||
| ctr.Env = append(ctr.Env, corev1.EnvVar{Name: k, Value: v}) | ||
| } | ||
| ctr.Env = append(ctr.Env, newSortedEnvVars(ctrConfig.EnvVars)...) |
There was a problem hiding this comment.
I ran into something similar to this in the POC as well but handled it in the resource level: https://github.com/sourcegraph/sourcegraph-operator/blob/main/internal/controller/resource/container/container.go#L87
Since introducing the standard feature for container env vars in https://github.com/sourcegraph/sourcegraph/pull/62624, some test flakiness has been introduced. This is because container.go was iterating through an unsorted map, which made golden file assertions non-deterministic. This is fixed by sorting the map keys when appending user-provided env vars.
Test plan
Ran
go test -count=10 -failfast ./internal/applianceand it passed. Before this commit, 10 runs would reliably tease out the flake.