Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: TestRefPathToGoType failing when shuffle tests #1446

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: TestRefPathToGoType failing when shuffle tests #1446

wants to merge 1 commit into from
+8 −3

Conversation

alexandear
Copy link
Contributor

@alexandear alexandear commented Jan 27, 2024 •
edited

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.

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 alexandear changed the title Fix failing tests when run with '-shuffle=on' Fix failing TestRefPathToGoType when run with '-shuffle=on' Jan 27, 2024
@alexandear alexandear changed the title Fix failing TestRefPathToGoType when run with '-shuffle=on' Fix failing TestRefPathToGoType when run tests with '-shuffle=on' Jan 27, 2024
@alexandear alexandear changed the title Fix failing TestRefPathToGoType when run tests with '-shuffle=on' fix: TestRefPathToGoType failed when run tests with '-shuffle=on' Jan 27, 2024
@alexandear alexandear changed the title fix: TestRefPathToGoType failed when run tests with '-shuffle=on' fix: TestRefPathToGoType failed when shuffle tests Jan 27, 2024
@alexandear alexandear changed the title fix: TestRefPathToGoType failed when shuffle tests fix: TestRefPathToGoType failing when shuffle tests Jan 27, 2024
@jamietanna
Copy link
Collaborator

@sonasingh46 mind a review?

@sonasingh46
Copy link
Contributor

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

@alexandear
Copy link
Contributor Author

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@alexandear @jamietanna @sonasingh46