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

log/slog: faulty test fails on spaces in file path #61161

Closed
merrickclay opened this issue Jul 3, 2023 · 5 comments
Closed

log/slog: faulty test fails on spaces in file path #61161

merrickclay opened this issue Jul 3, 2023 · 5 comments
Assignees
@jba
Labels
NeedsFix The path to resolution is known, but the work has not been done. release-blocker Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone
Go1.21

Comments

@merrickclay
Copy link
Contributor

merrickclay commented Jul 3, 2023 •
edited

What version of Go are you using (go version)?

Compiler go version go1.20.5 windows/amd64
Attempted to Build go version devel go1.21-5b72f45dd1 Fri Jun 30 22:02:00 2023 +0000 windows/amd64

Does this issue reproduce with the latest release?

slog is in next release

What operating system and processor architecture are you using (go env)?

go env Output
$ set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\Merrick Clay\AppData\Local\go-build
set GOENV=C:\Users\Merrick Clay\AppData\Roaming\go\env
set GOEXE=.exe
set GOEXPERIMENT=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\Merrick Clay\goprojects\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\Merrick Clay\goprojects
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org/,direct
set GOROOT=C:\Program Files\Go
set GOSUMDB=[sum.golang.org](http://sum.golang.org/)
set GOTMPDIR=
set GOTOOLDIR=C:\Program Files\Go\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=go1.20.5
set GCCGO=gccgo
set GOAMD64=v1
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=C:\Users\Merrick Clay\goprojects\src\goroot\src\go.mod
set GOWORK=
set CGO_CFLAGS=-O2 -g
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-O2 -g
set CGO_FFLAGS=-O2 -g
set CGO_LDFLAGS=-O2 -g
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=C:\Users\MERRIC~1\AppData\Local\Temp\go-build60902014=/tmp/go-build -gno-record-gcc-switches

What did you do?

  1. Downloaded source code
  2. cd src
  3. all.bat
  4. Error occurred in testing

What did you expect to see?

I expected all tests to pass

What did you see instead?

TestConnections failed 
--- FAIL: TestConnections (0.00s)
    logger_test.go:109:
        got  time=2023-07-02T18:04:14.412-06:00 level=INFO source="C:/Users/Merrick Clay/goprojects/src/goroot/src/log/slog/logger_test.go:108" msg=msg2
        want ^time=\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}(Z|[+-]\d{2}:\d{2}) level=INFO source=.*logger_test.go:\d{3} msg=msg2$
FAIL
FAIL    log/slog        1.187s

It appears the 'expected' regex does not anticipate the quotes that are added to the source when the path has a space in it (as it does in the Merrick Clay directory). Either the regex should be updated to add an optional quote after logger_test.go:\d{3 or slog should be updated to always or never surround the source in quotes (and the regex should be updated).

Updating the regex in the test by adding "? after logger_test.go:\d{3 allowed me to build and pass all tests. I'd love to put resolve this issue with a PR as my first contribution to the project once a path forward is agreed upon :)
@bcmills bcmills added Testing An issue that has been verified to require only test changes, not just a test failure. NeedsFix The path to resolution is known, but the work has not been done. release-blocker labels Jul 5, 2023
@bcmills bcmills added this to the Go1.21 milestone Jul 5, 2023
@bcmills
Copy link
Contributor

bcmills commented Jul 5, 2023

(attn @jba)

Marking as release-blocker for Go 1.21 because this is a new test failure in 1.21 and windows/amd64 is a first-class port.

@bcmills
Copy link
Contributor

bcmills commented Jul 5, 2023

@merrickclay, please feel free to send a fix per https://go.dev/doc/contribute.

(I would recommend sending directly through Gerrit instead of using a GitHub PR — the GitHub import flow is kind of rough and you're going to end up in Gerrit anyway.)

@jba
Copy link
Contributor

jba commented Jul 5, 2023

@merrickclay, let me know if you want to fix. If not, I'm happy to do it.

@dmitshur dmitshur changed the title log/slog: Faulty test fails on spaces in file path log/slog: faulty test fails on spaces in file path Jul 5, 2023
@merrickclay
Copy link
Contributor Author

@jba I'm walking through https://go.dev/doc/contribute to submit a fix :)

@gopherbot
Copy link

Change https://go.dev/cl/508055 mentions this issue: log/slog: fix faulty test

bradfitz pushed a commit to tailscale/go that referenced this issue Jul 15, 2023
@merrickclay @bradfitz
log/slog: fix faulty test
806eea9 
Adds an optional close quote in the expected log message regex for TestConnections to prevent failing when the source filepath is surrounded in quotes due to it containing one or more spaces.

Fixes golang#61161

Change-Id: I0dd71fb4389bff963bbfdc668ef4e4dfe787eafc
Reviewed-on: https://go-review.googlesource.com/c/go/+/508055
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
eric pushed a commit to fancybits/go that referenced this issue Sep 7, 2023
@merrickclay @eric
log/slog: fix faulty test
cabff39 
Adds an optional close quote in the expected log message regex for TestConnections to prevent failing when the source filepath is surrounded in quotes due to it containing one or more spaces.

Fixes golang#61161

Change-Id: I0dd71fb4389bff963bbfdc668ef4e4dfe787eafc
Reviewed-on: https://go-review.googlesource.com/c/go/+/508055
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
eric pushed a commit to fancybits/go that referenced this issue Sep 7, 2023
@merrickclay @eric
log/slog: fix faulty test
14dc1e7 
Adds an optional close quote in the expected log message regex for TestConnections to prevent failing when the source filepath is surrounded in quotes due to it containing one or more spaces.

Fixes golang#61161

Change-Id: I0dd71fb4389bff963bbfdc668ef4e4dfe787eafc
Reviewed-on: https://go-review.googlesource.com/c/go/+/508055
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jba jba

Labels
NeedsFix The path to resolution is known, but the work has not been done. release-blocker Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Milestone
Go1.21
Development

No branches or pull requests
4 participants
@bcmills @gopherbot @jba @merrickclay