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.

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

run.bash: document GO_TEST_SHORT and GO_TEST_TIMEOUT_SCALE; decide what to do with GOTESTONLY #46054

Closed
perillo opened this issue May 8, 2021 · 8 comments
Closed

run.bash: document GO_TEST_SHORT and GO_TEST_TIMEOUT_SCALE; decide what to do with GOTESTONLY #46054

perillo opened this issue May 8, 2021 · 8 comments
Assignees
@dmitshur
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.20

Comments

@perillo
Copy link
Contributor

perillo commented May 8, 2021

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

$ go version
go version go1.16.4 linux/amd64

Does this issue reproduce with the latest release?

Yes.

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

go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN="/home/manlio/.local/bin"
GOCACHE="/home/manlio/.cache/go-build"
GOENV="/home/manlio/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE="*.local"
GOMODCACHE="/home/manlio/.local/lib/go/pkg/mod"
GONOPROXY=""
GONOSUMDB="*.local"
GOOS="linux"
GOPATH="/home/manlio/.local/lib/go:/home/manlio/src/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.16.4"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build2218126867=/tmp/go-build -gno-record-gcc-switches"
GOROOT/bin/go version: go version go1.16.4 linux/amd64
GOROOT/bin/go tool compile -V: compile version go1.16.4
uname -sr: Linux 5.11.16-arch1-1
/usr/lib/libc.so.6: GNU C Library (GNU libc) release release version 2.33.
gdb --version: GNU gdb (GDB) 10.1

What did you do?

The run.bash script documents the GO_TEST_SHARDS and GO_BUILDER_NAME environment variables, but not the GOTESTONLY, GO_TEST_SHORT and GO_TEST_TIMEOUT_SCALE enviroment variables.
@heschi heschi added the NeedsFix The path to resolution is known, but the work has not been done. label May 10, 2021
@heschi heschi added this to the Unplanned milestone May 10, 2021
@heschi
Copy link
Contributor

heschi commented May 10, 2021

cc @rsc as a wild guess for an owner.

@bcmills
Copy link
Contributor

bcmills commented May 10, 2021

CC @golang/release, since the primary use-case for most of these variables is to configure the Go dashboard builders.

@dmitshur
Copy link
Contributor

Also CC @golang/release.

GO_TEST_SHORT is currently an internal implementation detail of the build system and not intended for users (see here). Before documenting it publicly, we'd need to decide that it should be made available to users (and supported as such). Issue #39054 may be relevant here.

I'm less familiar with GOTESTONLY but it looks like something made for builders (see CL 2523), given users can just specify a value by directly setting the -run flag. Although I suppose they can't if running cmd/dist indirectly via all.bash or so. I'm not sure if it should be made public, but if so, perhaps it should be renamed to be more consistent with the others (add underscore separators?).

GO_TEST_TIMEOUT_SCALE seems more generally useful, and perhaps worth documenting publicly.

Moving to NeedsDecision to decide whether to document these and consider them supported publicly, or document that they're an internal detail of the Go build system and not supported.

@dmitshur dmitshur added NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. and removed NeedsFix The path to resolution is known, but the work has not been done. labels May 10, 2021
@perillo
Copy link
Contributor Author

perillo commented May 10, 2021

FYI:

@gopherbot
Copy link

Change https://golang.org/cl/328771 mentions this issue: cmd/internal/moddeps: use a temporary directory for GOMODCACHE if needed

@dmitshur
Copy link
Contributor

By now I think it's okay to document GO_TEST_SHORT and GO_TEST_TIMEOUT_SCALE variables and their effect, even if they're meant to be used internally in the project and its build system. The documentation can convey that fact.

We should probably remove or rename GOTESTONLY, it's not actively used by the build system as far as I can tell, and its naming pattern is inconsistent. That should wait until we're not in the freeze.

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Jun 21, 2021
@dmitshur dmitshur changed the title run.bash: document GOTESTONLY, GO_TEST_SHORT and GO_TEST_TIMEOUT_SCALE run.bash: document GO_TEST_SHORT and GO_TEST_TIMEOUT_SCALE; decide what to do with GOTESTONLY Jun 21, 2021
gopherbot pushed a commit that referenced this issue Jun 21, 2021
cmd/internal/moddeps: use a temporary directory for GOMODCACHE if needed
761edf7 
CL 328770 should be sufficient to fix the specific failure in the
report, but when attempting to reproduce it I noticed a related
failure mode, triggered by the environment variables set in
src/run.bash.

The failure mode is currently masked on the Go project builders due to
the lack of any 'longtest' builder running as a non-root user
(#10719).

It is also masked from Go contributors running 'run.bash' locally
because 'run.bash' does not actually run all of the tests unless
GO_TEST_SHORT=0 is set in the environment (#29266, #46054).

Fixes #46695

Change-Id: I272c09dae462734590dce59b3d3c5b6d3f733c92
Reviewed-on: https://go-review.googlesource.com/c/go/+/328771
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
@gopherbot
Copy link

Change https://go.dev/cl/455519 mentions this issue: cmd/dist: remove GOTESTONLY environment variable

@gopherbot
Copy link

Change https://go.dev/cl/455517 mentions this issue: run.bash, cmd/dist: document GO_TEST_SHORT and GO_TEST_TIMEOUT_SCALE

@dmitshur dmitshur modified the milestones: Unplanned, Go1.20 Dec 6, 2022
@dmitshur dmitshur self-assigned this Dec 6, 2022
gopherbot pushed a commit that referenced this issue Jan 20, 2023
@dmitshur @gopherbot
cmd/dist: remove GOTESTONLY environment variable
e7495d8 
This hook was added for the Go build system (x/build) to be able to set
the run flag value, but it's no longer used anywhere. Remove it for now.

Updates #46054.

Change-Id: I64e7d68d2b270303f3bd54f73079600f209e350a
Reviewed-on: https://go-review.googlesource.com/c/go/+/455519
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Austin Clements <austin@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
@golang golang locked and limited conversation to collaborators Dec 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@dmitshur dmitshur

Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.20
Development

No branches or pull requests
5 participants
@dmitshur @bcmills @perillo @gopherbot @heschi