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

tools/internal: tool.Main should be refactored to simplify testing. #34291

Closed
hartzell opened this issue Sep 13, 2019 · 4 comments
Closed

tools/internal: tool.Main should be refactored to simplify testing. #34291

hartzell opened this issue Sep 13, 2019 · 4 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@hartzell
Copy link

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

(alice)[14:30:23]tools>>go version
go version go1.12.9 darwin/amd64
(alice)[14:40:51]tools>>

Does this issue reproduce with the latest release?

Yes

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

go env Output
(alice)[14:40:51]tools>>go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/hartzell/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/hartzell/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/Cellar/go/1.12.9/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.12.9/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/hartzell/tmp/golang/tools/go.mod"
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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/f7/fcb363190nz0jbypndp3nj_m0000gn/T/go-build454450712=/tmp/go-build -gno-record-gcc-switches -fno-common"
(alice)[14:41:13]tools>>

The issue

CL 194878 is making the GoPls rename functionality available from the command line. The rename test suite includes mostly successful tests (when run on this input you should get this output), but two cases are designed to generate errors, which are passed up the call chain.

tool.Main is the main entry point for the command line tools. It is explicitly intended to be called by application main() routines. If/when it encounters an error it prints the error to standard error and exists with a non-zero status (2).

tool.Main is also invoked internally and from various test functions. Exiting when the underlying functionality being exercised makes testing difficult, unnecessarily. For example, it causes false negatives (tests fail that shouldn't) in the the work-in-progress (https://go-review.googlesource.com/c/tools/+/194878/5) to implement gopls rename.

A simple fix is to break the functionality into two parts, calling the outer from main() functions and calling the inner bits internally and in tests.

I have a change ready that implements this.

@toothrot
Copy link
Contributor

/cc @ianthehat @stamblerre

@toothrot toothrot added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 17, 2019
@toothrot toothrot added this to the Go1.14 milestone Sep 17, 2019
@hartzell
Copy link
Author

@toothroot -- this is only used in gopackages and gopls. Is/should it be tied to a Go release (I'm always learning...)?

@toothrot
Copy link
Contributor

@hartzell Whoops! My mistake, thanks for catching this.

@toothrot toothrot modified the milestones: Go1.14, Unreleased Sep 18, 2019
@toothrot toothrot added the Tools This label describes issues relating to any tools in the x/tools repository. label Sep 18, 2019
@hartzell
Copy link
Author

This was fixed in cl195338.

@golang golang locked and limited conversation to collaborators Sep 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

3 participants