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

testing/slogtest: TestHandler passing tests that should fail #67605

Closed
mmTristan opened this issue May 23, 2024 · 2 comments
Closed

testing/slogtest: TestHandler passing tests that should fail #67605

mmTristan opened this issue May 23, 2024 · 2 comments
Labels
FixPending Issues that have a fix which has not yet been reviewed or submitted. NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.24

Comments

@mmTristan
Copy link

Go version

go version go1.22.0 linux/amd64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/usr/.cache/go-build'
GOENV='/home/usr/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/usr/go/pkg/mod'
GONOPROXY='gitlab.com/*,github.com/*'
GONOSUMDB='gitlab.com/*,github.com/*'
GOOS='linux'
GOPATH='/home/usr/go'
GOPRIVATE='gitlab.com/*,github.com/*'
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.22.0'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/mnt/c/Users/usr/Documents/git/glmrx-svr-dev/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build3234934194=/tmp/go-build -gno-record-gcc-switches'

What did you do?

I was trying to implement slog test handlers based off of the example code. But when checking for the expected errors with parsing empty maps, it still passes the test.
Here is the go playground of the error (or lack of)

It looks like in the source code at line 219 it calls the test again without checking for the length of the returned slice. So when a 0 length slice is returned the tests aren't run.

There is a workaround where you put the results back into the channel, but that is easy to overloook. workaround go playground
The code should be updated to check for the lengths again, or use the results from the length checking test call.

What did you see happen?

Ran the test and got this response.

test errors: <nil>

What did you expect to see?

Expected to get a list of missing keys in the returned maps.

test errors: A lot of errors for each map key
@mmTristan mmTristan changed the title slog/slogtest Testhandler passing tests that should fail slog/slogtest: Testhandler passing tests that should fail May 23, 2024
@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 24, 2024
@cagedmantis cagedmantis added this to the Backlog milestone May 24, 2024
@cagedmantis
Copy link
Contributor

cc @jba

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/599836 mentions this issue: testing/slogtest: reuse results obtained from previous call

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. FixPending Issues that have a fix which has not yet been reviewed or submitted. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jul 22, 2024
@dmitshur dmitshur changed the title slog/slogtest: Testhandler passing tests that should fail slog/slogtest: TestHandler passing tests that should fail Jul 22, 2024
@dmitshur dmitshur changed the title slog/slogtest: TestHandler passing tests that should fail testing/slogtest: TestHandler passing tests that should fail Jul 22, 2024
@dmitshur dmitshur modified the milestones: Backlog, Go1.24 Nov 19, 2024
This was referenced Feb 20, 2025
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
FixPending Issues that have a fix which has not yet been reviewed or submitted. NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.24
Development

No branches or pull requests
4 participants
@cagedmantis @dmitshur @gopherbot @mmTristan