Skip to content

x/build/cmd/coordinator: compile error in os/os_test.go not detected by misc-compile TryBots #61923

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
Closed
bcmills opened this issue Aug 10, 2023 · 6 comments
Closed

x/build/cmd/coordinator: compile error in os/os_test.go not detected by misc-compile TryBots #61923

bcmills opened this issue Aug 10, 2023 · 6 comments
Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Unreleased

Comments

@bcmills
Copy link
Contributor

bcmills commented Aug 10, 2023

https://go.dev/cl/517755 caused a compilation error on the plan9 builders (https://build.golang.org/log/2613479c07e3aaf02e75f15f00b8d1c01c9ec217):

# os_test [os.test]
os/os_test.go:3297:70: too many arguments in call to syscall.Open
	have (string, number, number)
	want (string, int)
os/os_test.go:3302:55: too many arguments in call to syscall.Open
	have (string, number, number)
	want (string, int)
FAIL	os [build failed]

I didn't catch that during code review because the CL had a passing TryBot run, and based on #20119 I assumed that the misc-compile TryBots would have caught any compile error in the test.

It appears that either the fix for #20119 was incomplete, or something has regressed since then to cause the misc-compile TryBots to no longer catch compile errors in tests.

(CC @dmitshur @aclements @golang/release)
@bcmills bcmills added Builders x/build issues (builders, bots, dashboards) NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Aug 10, 2023
@gopherbot gopherbot added this to the Unreleased milestone Aug 10, 2023
@bcmills
Copy link
Contributor Author

bcmills commented Aug 10, 2023

I suspect this regressed in https://go.dev/cl/464955 (for #58163; attn @mknyszek).

@mknyszek
Copy link
Contributor

Thanks for flagging this, on it.

@mknyszek
Copy link
Contributor

OK yeah, got it. I suspect this should be an easy fix.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/518255 mentions this issue: os: define TestIssue60181 only on Unix platforms

@dmitshur
Copy link
Member

For the record, this is is being caught in LUCI as intended, see the misccompile column at https://ci.chromium.org/p/golang/g/go-gotip-ci/console. So this issue only affects x/build/cmd/coordinator.

image

@dmitshur dmitshur changed the title x/build: compile error in os/os_test.go not detected by misc-compile TryBots x/build/cmd/coordinator: compile error in os/os_test.go not detected by misc-compile TryBots Aug 10, 2023
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/518295 mentions this issue: cmd/coordinator: build test packages for misc-compile builders

gopherbot pushed a commit that referenced this issue Aug 10, 2023
@gopherbot
os: define TestIssue60181 only on Unix platforms
e34de96 
In CL 517755 the test was added in the unconstrained os_test.go
because it appeared to be portable, but it turned out not to be
valid on plan9.

(The build error was masked on the misc-compile TryBots by #61923.)

Although the test can also compile and run on Windows, the bug it
checks for is specific to Linux and only really needs to run there, so
I am moving it to os_unix_test.go instead of adding yet another test
file for “Unix and Windows but not Plan 9”.

Updates #60181.

Change-Id: I41fd11b288217e95652b5daa72460c0d26bde606
Reviewed-on: https://go-review.googlesource.com/c/go/+/518255
Auto-Submit: Bryan Mills <[email protected]>
Reviewed-by: Tobias Klauser <[email protected]>
Run-TryBot: Bryan Mills <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Aug 10, 2023
@dmitshur dmitshur moved this to In Progress in Go Release Aug 15, 2023
@github-project-automation github-project-automation bot moved this from In Progress to Done in Go Release Aug 16, 2023
@golang golang locked and limited conversation to collaborators Aug 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
Go Release
Archived in project
Milestone
Unreleased
Development

No branches or pull requests
4 participants
@mknyszek @dmitshur @bcmills @gopherbot