instrumenta / kubeval

Validate your Kubernetes configuration files, supports multiple Kubernetes versions
https://kubeval.com
Other
3.16k stars 229 forks source link

unit tests change global flag which persists for other unit tests #141

Closed jeffg-hpe closed 5 years ago

jeffg-hpe commented 5 years ago

If a unit test modifies the flag Strict , that change persists to other unit tests, resulting in unexpected behavior.

The end result might be a non-issue currently due to what is being tested, but it's non-intuitive for devs adding additional functionality and writing similar tests (see #127).

These unit tests are the only ones that modify Strict:

kubeval/kubeval_test.go-func TestStrictCatchesAdditionalErrors(t *testing.T) {
kubeval/kubeval_test.go:        Strict = true
--
kubeval/kubeval_test.go-func TestValidateMultipleVersions(t *testing.T) {
kubeval/kubeval_test.go:        Strict = true
--
kubeval/kubeval_test.go-func TestDetermineSchema(t *testing.T) {
kubeval/kubeval_test.go:        Strict = false

When I run make test, a different test TestValidInputsWithErrors which does not set Strict, and thus probably expects it to be false, actually runs with Strict = true:

func TestValidateInputsWithErrors(t *testing.T) {
    if Strict {
        t.Errorf("I didn't enable strict mode")
    }
$ make test
...
=== RUN   TestValidateInputsWithErrors
--- FAIL: TestValidateInputsWithErrors (0.31s)
    kubeval_test.go:107: I didn't enable strict mode
...

I'd propose considering fixing this by passing in Strict and similar flags to the Validate, ValidateWithCache, and determineSchema, but that's just a suggestion.

garethr commented 5 years ago

Agreed. I've been aware of this for a while and haven't had enough time to refactor it out. Passing in the flags is definitely the right approach I think.

garethr commented 5 years ago

This should be resolved by #158