Hmm, I'm now much less convinced about how feasible this is. "go vet" needs to have all imported packages available to it, so we'd probably need to set $GOPATH to include the tast and tast-tests repositories and /usr/lib/gopath in the chroot when running it. I think it may do compilation itself, which would happen using a random toolchain when running outside of the chroot. And so on.

Maybe it'd be better to run this as part of compilation instead.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/tast-tests/+/66428c64d22ceb5dc68c183c2e105cedb90d57e0

commit 66428c64d22ceb5dc68c183c2e105cedb90d57e0
Author: Daniel Erat <derat@chromium.org>
Date: Tue Sep 25 00:49:39 2018

tast-tests: Fix go vet warnings in tests.

Add a screenshot.RGB function and make the
graphics.Screenshot test call it to avoid a "composite
literal uses unkeyed fields" error from go vet. The
alternative would be calling "screenshot.Color{R: 123, G:
123, B: 123}" everywhere.

Also fix a Fatal call that should be Fatalf in the
subtest.VerifyAppFromTerminal function called by
vm.CrostiniStartEverything.

BUG= chromium:888262 , chromium:888259 
TEST=screenshot test still passes and "go vet" no longer
     complains

Change-Id: I4b481f6a3a01f2292188488d84d3579f2a4019e4
Reviewed-on: https://chromium-review.googlesource.com/1239191
Commit-Ready: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: Jeffrey Kardatzke <jkardatzke@google.com>

[modify] https://crrev.com/66428c64d22ceb5dc68c183c2e105cedb90d57e0/src/chromiumos/tast/local/bundles/cros/graphics/screenshot.go
[modify] https://crrev.com/66428c64d22ceb5dc68c183c2e105cedb90d57e0/src/chromiumos/tast/local/bundles/cros/vm/subtest/verify_app_from_terminal.go
[modify] https://crrev.com/66428c64d22ceb5dc68c183c2e105cedb90d57e0/src/chromiumos/tast/local/screenshot/screenshot.go

https://crrev.com/c/1239657 adds CROS_GO_VET and CROS_GO_VET_FLAGS variables to cros-go.eclass that can be used to configure how "go vet" runs during src_compile.

The docs at https://golang.org/cmd/vet/ describe a -source flag:

  By default vet uses the object files generated by 'go install some/pkg'
  to typecheck the code. If the -source flag is provided, vet uses only
  source code.

I was hoping this would make the command only inspect source file(s) passed to it, allowing it to be easily run from pre-upload hooks, but unfortunately it still tries to resolve imports:

  % pwd
  .../chromeos/src/platform/tast/src/chromiumos/tast/runner
  % go vet -source runner.go
  runner.go:19:2: cannot find package "chromiumos/tast/bundle" in any of:
  ...

And it's clearly also parsing those imports and trying to resolve their imports as well:

  % GOPATH=$HOME/chromeos/src/platform/tast go vet -source runner.go
  ../host/ssh.go:26:2: cannot find package "golang.org/x/crypto/ssh" in any of:
  ...

Fixing my embarrassing cc mistake. Sorry. :-(

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/tast/+/c96b879a49f20ffe4ef6e542cb9d7ef0d0f3fbcb

commit c96b879a49f20ffe4ef6e542cb9d7ef0d0f3fbcb
Author: Daniel Erat <derat@chromium.org>
Date: Wed Sep 26 01:57:04 2018

tast: Make fast_build.sh pass -printfuncs to "go vet".

Make the "fast_build.sh -C" pass a -printfuncs flag to "go
vet" instructing it to check calls to Log/Logf etc. methods
on testing.State.

BUG= chromium:888259 , chromium:814327 
TEST=ran "./fast_build.sh -C" in the chroot

Change-Id: I936636a4f15409ea3046da139946f7bbf38593ec
Reviewed-on: https://chromium-review.googlesource.com/1244422
Tested-by: Dan Erat <derat@chromium.org>
Trybot-Ready: Dan Erat <derat@chromium.org>
Reviewed-by: Shuhei Takahashi <nya@chromium.org>

[modify] https://crrev.com/c96b879a49f20ffe4ef6e542cb9d7ef0d0f3fbcb/fast_build.sh

 Issue 889391  has been merged into this issue.

From hidehiko@: "Context:
https://chromium-review.googlesource.com/c/chromiumos/platform/tast-tests/+/1235874/6/src/chromiumos/tast/local/bundles/cros/security/open_fds.go#44

go vet can fail. It will be nice if we can run it in preupload hook, because developers can know the failure quickly.

Local experiment:

cd ${chromeosroot}/platform/tast-tests
GOPATH=${chromeosroot}/chroot/usr/lib/gopath:${chromeosroot}/src/platform/tast:${chromeosroot}/platform/tast-tests go vet chromiumos/tast/local/...

worked successfully on my local dev env at least."

I initially tried to write a pre-upload hook for this, but "go vet" appears to compile code, so I was concerned about using a different Go toolchain from what we use in the chroot. Maybe that's not an issue if we pass -source, though.

My other concern was that I think that this requires that all of the tast-{local,remote}-test-cros packages' dependencies have been built recently for the host sysroot, since that's how source gets installed in (.../chroot/usr/lib/gopath. This may be confusing for developers, so I figured it was safer to try to stay as close to the normal build environment as possible.

Hopefully I'm wrong about some of the above. It'd be very nice to do this in a pre-upload hook instead of as a compile-time check.

I'm closing this, but please feel free to reopen it if there's a reliable way to run this from a preupload hook (e.g. maybe there's some way that I couldn't find to convince "go vet" to not follow imports).