oapi-codegen icon indicating copy to clipboard operation
oapi-codegen copied to clipboard

fix: TestRefPathToGoType failing when shuffle tests

Open alexandear opened this issue 2 years ago • 8 comments

This PR fixes TestRefPathToGoType which failed when test shuffling enabled:

	-shuffle off,on,N
	    Randomize the execution order of tests and benchmarks.
	    It is off by default. If -shuffle is set to on, then it will seed
	    the randomizer using the system clock. If -shuffle is set to an
	    integer N, then N will be used as the seed value. In both cases,
	    the seed will be reported for reproducibility.

Additionally, you can reproduce panic by running the command:

go test  -run ^TestRefPathToGoType ./...
The full error
+ go test -shuffle=on -cover ./...
?       github.com/deepmap/oapi-codegen/v2/pkg/ecdsafile        [no test files]
?       github.com/deepmap/oapi-codegen/v2/pkg/securityprovider [no test files]
ok      github.com/deepmap/oapi-codegen/v2/cmd/oapi-codegen     1.030s  coverage: 0.0% of statements
-test.shuffle 1706383600410948000
--- FAIL: TestRefPathToGoType (0.00s)
    --- FAIL: TestRefPathToGoType/local-schemas (0.00s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
        panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x18 pc=0x104e1d378]

goroutine 37 [running]:
testing.tRunner.func1.2({0x104fd7280, 0x1053b0d40})
        /opt/homebrew/Cellar/go/1.21.5/libexec/src/testing/testing.go:1545 +0x1c4
testing.tRunner.func1()
        /opt/homebrew/Cellar/go/1.21.5/libexec/src/testing/testing.go:1548 +0x360
panic({0x104fd7280?, 0x1053b0d40?})
        /opt/homebrew/Cellar/go/1.21.5/libexec/src/runtime/panic.go:914 +0x218
github.com/deepmap/oapi-codegen/v2/pkg/codegen.findSchemaNameByRefPath({0x104e540ee?, 0x18?}, 0x104f5a6d0?)
        /Users/Oleksandr_Redko/src/github.com/deepmap/oapi-codegen/pkg/codegen/utils.go:892 +0x48
github.com/deepmap/oapi-codegen/v2/pkg/codegen.refPathToGoType({0x104e540ee, 0x18}, 0x1)
        /Users/Oleksandr_Redko/src/github.com/deepmap/oapi-codegen/pkg/codegen/utils.go:388 +0x23c
github.com/deepmap/oapi-codegen/v2/pkg/codegen.RefPathToGoType(...)
        /Users/Oleksandr_Redko/src/github.com/deepmap/oapi-codegen/pkg/codegen/utils.go:370
github.com/deepmap/oapi-codegen/v2/pkg/codegen.TestRefPathToGoType.func2(0x0?)
        /Users/Oleksandr_Redko/src/github.com/deepmap/oapi-codegen/pkg/codegen/utils_test.go:276 +0x64
testing.tRunner(0x140002181a0, 0x14000119340)
        /opt/homebrew/Cellar/go/1.21.5/libexec/src/testing/testing.go:1595 +0xe8
created by testing.(*T).Run in goroutine 36
        /opt/homebrew/Cellar/go/1.21.5/libexec/src/testing/testing.go:1648 +0x33c
FAIL    github.com/deepmap/oapi-codegen/v2/pkg/codegen  0.522s
ok      github.com/deepmap/oapi-codegen/v2/pkg/util     0.540s  coverage: 55.6% of statements
FAIL

alexandear avatar Jan 27 '24 19:01 alexandear

@sonasingh46 mind a review?

jamietanna avatar Jan 29 '24 08:01 jamietanna

@alexandear -- Do you know why the original code panicked when shuffle was enabled?

sonasingh46 avatar Jan 30 '24 09:01 sonasingh46

@sonasingh46, I believe I've found the reason why the original test code was causing a panic.

The TestRefPathToGoType/local-schemas test seems to panic when globalState.spec is nil. Under normal circumstances, without shuffling, tests are executed in a specific order. There's a test named TestGoTypeImport that runs before TestRefPathToGoType and sets globalState.spec to a non-nil value. This happens because TestGoTypeImport invokes the Generate() function, which internally assigns a value to globalState.spec.

Without shuffling TestGoTypeImport runs before TestRefPathToGoType:

❯ go test -race -failfast -v pkg/codegen/*.go
...
=== RUN   TestGoTypeImport
--- PASS: TestGoTypeImport (0.06s)
...
--- PASS: TestRefPathToGoType (0.00s)
    --- PASS: TestRefPathToGoType/local-schemas (0.00s)
...

Without shuffling TestGoTypeImport might be after TestRefPathToGoType:

❯ go test -race -shuffle=on -failfast -v pkg/codegen/*.go
-test.shuffle 1706733905370661000
...
=== RUN   TestRefPathToGoType
=== RUN   TestRefPathToGoType/local-schemas
--- FAIL: TestRefPathToGoType (0.00s)
    --- FAIL: TestRefPathToGoType/local-schemas (0.00s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
        panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x0 pc=0x100992dc4]

I hope this explanation clarifies the issue.

alexandear avatar Jan 31 '24 21:01 alexandear

@jamietanna @sonasingh46 could you please review?

alexandear avatar May 25 '24 09:05 alexandear

I'll look at it after I get round to the v2.2.0 release, which has a few other pressing issues - thanks for the fix!

jamietanna avatar May 25 '24 10:05 jamietanna