Add t.Helper calls, testing linter (#1534)

Add `t.Helper()` calls to testing helpers so the correct test name and line number is shown during logs [ref](https://pkg.go.dev/testing#B.Helper). Add [`thelper`](https://github.com/kulti/thelper) linter to settings to make sure testing helpers correctly call `t.Helper`. Renamed `t testing.TB` to `tb testing.TB` to satisfy `thelper`. Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
microsoft · Oct 6, 2022 · a6859d9 · a6859d9
1 parent 7fe1fa6
commit a6859d9
Show file tree

Hide file tree

Showing 89 changed files with 510 additions and 332 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -11,9 +11,19 @@ run:
 
 linters:
  enable:
- - gofmt
- - nolintlint
- - stylecheck
+ # defaults:
+ # - errcheck
+ # - gosimple
+ # - govet
+ # - ineffassign
+ # - staticcheck
+ # - typecheck
+ # - unused
+
+ - gofmt # whether code was gofmt-ed
+ - nolintlint # ill-formed or insufficient nolint directives
+ - stylecheck # golint replacement
+ - thelper # test helpers without t.Helper()
 
 linters-settings:
  stylecheck:

diff --git a/cmd/containerd-shim-runhcs-v1/exec_wcow_podsandbox_test.go b/cmd/containerd-shim-runhcs-v1/exec_wcow_podsandbox_test.go
@@ -14,6 +14,7 @@ import (
 )
 
 func verifyWcowPodSandboxExecStatus(t *testing.T, wasStarted bool, es containerd_v1_types.Status, status *task.StateResponse) {
+ t.Helper()
  if status.ID != t.Name() {
  t.Fatalf("expected id: '%s' got: '%s'", t.Name(), status.ID)
  }

diff --git a/cmd/containerd-shim-runhcs-v1/pod_test.go b/cmd/containerd-shim-runhcs-v1/pod_test.go
@@ -89,6 +89,7 @@ func (tsp *testShimPod) DeleteTask(ctx context.Context, tid string) error {
 // Pod tests
 
 func setupTestPodWithFakes(t *testing.T) (*pod, *testShimTask) {
+ t.Helper()
  st := &testShimTask{
  id: t.Name(),
  exec: newTestShimExec(t.Name(), t.Name(), 10),
@@ -105,6 +106,7 @@ func setupTestPodWithFakes(t *testing.T) (*pod, *testShimTask) {
 }
 
 func setupTestTaskInPod(t *testing.T, p *pod) *testShimTask {
+ t.Helper()
  tid := strconv.Itoa(rand.Int())
  wt := &testShimTask{
  id: tid,

diff --git a/cmd/containerd-shim-runhcs-v1/service_internal_podshim_test.go b/cmd/containerd-shim-runhcs-v1/service_internal_podshim_test.go
@@ -19,6 +19,7 @@ import (
 )
 
 func setupPodServiceWithFakes(t *testing.T) (*service, *testShimTask, *testShimTask, *testShimExec) {
+ t.Helper()
  tid := strconv.Itoa(rand.Int())
 
  s, err := NewService(WithTID(tid), WithIsSandbox(true))

diff --git a/cmd/containerd-shim-runhcs-v1/service_internal_taskshim_test.go b/cmd/containerd-shim-runhcs-v1/service_internal_taskshim_test.go
@@ -19,6 +19,7 @@ import (
 )
 
 func setupTaskServiceWithFakes(t *testing.T) (*service, *testShimTask, *testShimExec) {
+ t.Helper()
  tid := strconv.Itoa(rand.Int())
 
  s, err := NewService(WithTID(tid), WithIsSandbox(false))

diff --git a/cmd/containerd-shim-runhcs-v1/service_internal_test.go b/cmd/containerd-shim-runhcs-v1/service_internal_test.go
@@ -16,6 +16,7 @@ import (
 )
 
 func verifyExpectedError(t *testing.T, resp interface{}, actual, expected error) {
+ t.Helper()
  if actual == nil || errors.Cause(actual) != expected || !errors.Is(actual, expected) {
  t.Fatalf("expected error: %v, got: %v", expected, actual)
  }
@@ -33,6 +34,7 @@ func verifyExpectedError(t *testing.T, resp interface{}, actual, expected error)
 }
 
 func verifyExpectedStats(t *testing.T, isWCOW, ownsHost bool, s *stats.Statistics) {
+ t.Helper()
  if isWCOW {
  verifyExpectedWindowsContainerStatistics(t, s.GetWindows())
  } else {
@@ -44,6 +46,7 @@ func verifyExpectedStats(t *testing.T, isWCOW, ownsHost bool, s *stats.Statistic
 }
 
 func verifyExpectedWindowsContainerStatistics(t *testing.T, w *stats.WindowsContainerStatistics) {
+ t.Helper()
  if w == nil {
  t.Fatal("expected non-nil WindowsContainerStatistics")
  }
@@ -92,6 +95,7 @@ func verifyExpectedWindowsContainerStatistics(t *testing.T, w *stats.WindowsCont
 }
 
 func verifyExpectedCgroupMetrics(t *testing.T, v *v1.Metrics) {
+ t.Helper()
  if v == nil {
  t.Fatal("expected non-nil cgroups Metrics")
  }
@@ -116,6 +120,7 @@ func verifyExpectedCgroupMetrics(t *testing.T, v *v1.Metrics) {
 }
 
 func verifyExpectedVirtualMachineStatistics(t *testing.T, v *stats.VirtualMachineStatistics) {
+ t.Helper()
  if v == nil {
  t.Fatal("expected non-nil VirtualMachineStatistics")
  }

diff --git a/cmd/containerd-shim-runhcs-v1/task_hcs_test.go b/cmd/containerd-shim-runhcs-v1/task_hcs_test.go
@@ -13,6 +13,7 @@ import (
 )
 
 func setupTestHcsTask(t *testing.T) (*hcsTask, *testShimExec, *testShimExec) {
+ t.Helper()
  initExec := newTestShimExec(t.Name(), t.Name(), int(rand.Int31()))
  lt := &hcsTask{
  events: newFakePublisher(),
@@ -127,6 +128,7 @@ func Test_hcsTask_KillExec_2ndExecID_All_Error(t *testing.T) {
 }
 
 func verifyDeleteFailureValues(t *testing.T, pid int, status uint32, at time.Time) {
+ t.Helper()
  if pid != 0 {
  t.Fatalf("pid expected '0' got: '%d'", pid)
  }
@@ -139,6 +141,7 @@ func verifyDeleteFailureValues(t *testing.T, pid int, status uint32, at time.Tim
 }
 
 func verifyDeleteSuccessValues(t *testing.T, pid int, status uint32, at time.Time, e *testShimExec) {
+ t.Helper()
  if pid != e.pid {
  t.Fatalf("pid expected '%d' got: '%d'", e.pid, pid)
  }

diff --git a/cmd/gcstools/generichook.go b/cmd/gcstools/generichook.go
@@ -73,7 +73,6 @@ func logDebugFile(debugFilePath string) {
  i += 1
  startBytes += chunkSize
  }
-
 }
 
 func genericHookMain() {

diff --git a/ext4/dmverity/dmverity_test.go b/ext4/dmverity/dmverity_test.go
@@ -13,6 +13,7 @@ import (
 )
 
 func tempFileWithContentLength(t *testing.T, length int) *os.File {
+ t.Helper()
  tmpFile, err := os.CreateTemp("", "")
  if err != nil {
  t.Fatalf("failed to create temp file")

diff --git a/ext4/internal/compactext4/compact_test.go b/ext4/internal/compactext4/compact_test.go
@@ -64,6 +64,7 @@ func (tf *testFile) Reader() io.Reader {
 }
 
 func createTestFile(t *testing.T, w *Writer, tf testFile) {
+ t.Helper()
  var err error
  if tf.File != nil {
  tf.File.Size = int64(len(tf.Data))
@@ -136,6 +137,7 @@ func fileEqual(f1, f2 *File) bool {
 }
 
 func runTestsOnFiles(t *testing.T, testFiles []testFile, opts ...Option) {
+ t.Helper()
  image := "testfs.img"
  imagef, err := os.Create(image)
  if err != nil {

diff --git a/ext4/internal/compactext4/verify_linux_test.go b/ext4/internal/compactext4/verify_linux_test.go
@@ -134,7 +134,6 @@ func verifyTestFile(t *testing.T, mountPath string, tf testFile) {
  !timeEqual(st.Atim, tf.File.Atime) ||
  !timeEqual(st.Mtim, tf.File.Mtime) ||
  !timeEqual(st.Ctim, tf.File.Ctime) {
-
  t.Errorf("%s: stat mismatch, expected: %#v got: %#v", tf.Path, tf.File, st)
  }
 

diff --git a/ext4/internal/compactext4/verify_test.go b/ext4/internal/compactext4/verify_test.go
@@ -6,14 +6,18 @@ package compactext4
 import "testing"
 
 func verifyTestFile(t *testing.T, mountPath string, tf testFile) {
+ t.Helper()
 }
 
 func mountImage(t *testing.T, image string, mountPath string) bool {
+ t.Helper()
  return false
 }
 
 func unmountImage(t *testing.T, mountPath string) {
+ t.Helper()
 }
 
 func fsck(t *testing.T, image string) {
+ t.Helper()
 }
diff --git a/hcn/hcnendpoint_test.go b/hcn/hcnendpoint_test.go
@@ -362,5 +362,4 @@ func TestApplyTierAclPolicyOnEndpoint(t *testing.T) {
  if len(foundEndpoint.Policies) == 0 {
  t.Fatal("No Endpoint Policies found")
  }
-
 }
diff --git a/hcn/hcnnamespace_test.go b/hcn/hcnnamespace_test.go
@@ -13,6 +13,7 @@ import (
 )
 
 func newGUID(t *testing.T) guid.GUID {
+ t.Helper()
  g, err := guid.NewV4()
  if err != nil {
  t.Fatal(err)

diff --git a/hcn/hcnnetwork_test.go b/hcn/hcnnetwork_test.go
@@ -36,6 +36,7 @@ func TestCreateDeleteNetworks(t *testing.T) {
 }
 
 func CreateDeleteNetworksHelper(t *testing.T, networkFunction HcnNetworkMakerFunc) error {
+ t.Helper()
  network, err := networkFunction()
  if err != nil {
  return err
@@ -96,6 +97,7 @@ func TestListNetwork(t *testing.T) {
 }
 
 func testNetworkPolicy(t *testing.T, policiesToTest *PolicyNetworkRequest) {
+ t.Helper()
  network, err := CreateTestOverlayNetwork()
  if err != nil {
  t.Fatal(err)
@@ -150,7 +152,6 @@ func testNetworkPolicy(t *testing.T, policiesToTest *PolicyNetworkRequest) {
 }
 
 func TestAddRemoveRemoteSubnetRoutePolicy(t *testing.T) {
-
  remoteSubnetRoutePolicy, err := HcnCreateTestRemoteSubnetRoute()
  if err != nil {
  t.Fatal(err)
@@ -160,7 +161,6 @@ func TestAddRemoveRemoteSubnetRoutePolicy(t *testing.T) {
 }
 
 func TestAddRemoveHostRoutePolicy(t *testing.T) {
-
  hostRoutePolicy, err := HcnCreateTestHostRoute()
  if err != nil {
  t.Fatal(err)
@@ -170,18 +170,15 @@ func TestAddRemoveHostRoutePolicy(t *testing.T) {
 }
 
 func TestAddRemoveNetworACLPolicy(t *testing.T) {
-
  networkACLPolicy, err := HcnCreateNetworkACLs()
  if err != nil {
  t.Fatal(err)
  }
 
  testNetworkPolicy(t, networkACLPolicy)
-
 }
 
 func TestNetworkFlags(t *testing.T) {
-
  network, err := CreateTestOverlayNetwork()
  if err != nil {
  t.Fatal(err)

diff --git a/hcn/hcnutils_test.go b/hcn/hcnutils_test.go
@@ -401,11 +401,9 @@ func HcnCreateTestL2BridgeNetwork() (*HostComputeNetwork, error) {
  }
 
  return network.Create()
-
 }
 
 func HcnCreateTierAcls() (*PolicyEndpointRequest, error) {
-
  policy := make([]EndpointPolicy, 6)
 
  tiers := make([]TierAclPolicySetting, 6)

diff --git a/hcn/hnsv1_test.go b/hcn/hnsv1_test.go
@@ -37,7 +37,6 @@ func CreateTestNetwork() (*hcsshim.HNSNetwork, error) {
 }
 
 func TestEndpoint(t *testing.T) {
-
  network, err := CreateTestNetwork()
  if err != nil {
  t.Fatal(err)

diff --git a/internal/cni/registry_test.go b/internal/cni/registry_test.go
@@ -10,6 +10,7 @@ import (
 )
 
 func newGUID(t *testing.T) guid.GUID {
+ t.Helper()
  g, err := guid.NewV4()
  if err != nil {
  t.Fatal(err)

diff --git a/internal/gcs/bridge_test.go b/internal/gcs/bridge_test.go
@@ -34,6 +34,7 @@ func pipeConn() (*stitched, *stitched) {
 }
 
 func sendMessage(t *testing.T, w io.Writer, typ msgType, id int64, msg []byte) {
+ t.Helper()
  var h [16]byte
  binary.LittleEndian.PutUint32(h[:], uint32(typ))
  binary.LittleEndian.PutUint32(h[4:], uint32(len(msg)+16))
@@ -51,6 +52,7 @@ func sendMessage(t *testing.T, w io.Writer, typ msgType, id int64, msg []byte) {
 }
 
 func reflector(t *testing.T, rw io.ReadWriteCloser, delay time.Duration) {
+ t.Helper()
  defer rw.Close()
  for {
  id, typ, msg, err := readMessage(rw)
@@ -77,6 +79,7 @@ type testResp struct {
 }
 
 func startReflectedBridge(t *testing.T, delay time.Duration) *bridge {
+ t.Helper()
  s, c := pipeConn()
  b := newBridge(s, nil, logrus.NewEntry(logrus.StandardLogger()))
  b.Start()
@@ -149,6 +152,7 @@ func TestBridgeRPCBridgeClosed(t *testing.T) {
 }
 
 func sendJSON(t *testing.T, w io.Writer, typ msgType, id int64, msg interface{}) error {
+ t.Helper()
  msgb, err := json.Marshal(msg)
  if err != nil {
  return err
@@ -158,6 +162,7 @@ func sendJSON(t *testing.T, w io.Writer, typ msgType, id int64, msg interface{})
 }
 
 func notifyThroughBridge(t *testing.T, typ msgType, msg interface{}, fn notifyFunc) error {
+ t.Helper()
  s, c := pipeConn()
  b := newBridge(s, fn, logrus.NewEntry(logrus.StandardLogger()))
  b.Start()

diff --git a/internal/gcs/guestconnection_test.go b/internal/gcs/guestconnection_test.go
@@ -36,6 +36,7 @@ func dialPort(port uint32) (net.Conn, error) {
 }
 
 func simpleGcs(t *testing.T, rwc io.ReadWriteCloser) {
+ t.Helper()
  defer rwc.Close()
  err := simpleGcsLoop(t, rwc)
  if err != nil {
@@ -44,6 +45,7 @@ func simpleGcs(t *testing.T, rwc io.ReadWriteCloser) {
 }
 
 func simpleGcsLoop(t *testing.T, rw io.ReadWriter) error {
+ t.Helper()
  for {
  id, typ, b, err := readMessage(rw)
  if err != nil {
@@ -142,6 +144,7 @@ func simpleGcsLoop(t *testing.T, rw io.ReadWriter) error {
 }
 
 func connectGcs(ctx context.Context, t *testing.T) *GuestConnection {
+ t.Helper()
  s, c := pipeConn()
  if ctx != context.Background() && ctx != context.TODO() {
  go func() {

diff --git a/internal/guest/bridge/bridge_unit_test.go b/internal/guest/bridge/bridge_unit_test.go
@@ -168,6 +168,7 @@ func Test_Bridge_Mux_Handler_NilRequest_Panic(t *testing.T) {
 }
 
 func verifyResponseIsDefaultHandler(t *testing.T, resp RequestResponse) {
+ t.Helper()
  if resp == nil {
  t.Fatal("The response is nil")
  }

diff --git a/internal/guest/network/netns.go b/internal/guest/network/netns.go
@@ -178,7 +178,6 @@ func NetNSConfig(ctx context.Context, ifStr string, nsPid int, adapter *prot.Net
  if err := netlink.RouteAdd(&route); err != nil {
  return errors.Wrapf(err, "netlink.RouteAdd(%#v) failed", route)
  }
-
  }
  }
  } else {

diff --git a/internal/guest/runtime/hcsv2/nvidia_utils.go b/internal/guest/runtime/hcsv2/nvidia_utils.go
@@ -79,7 +79,6 @@ func addNvidiaDeviceHook(ctx context.Context, spec *oci.Spec) error {
 // gcstool's `install-drivers` binary.
 func getNvidiaDriversUsrLibPath() string {
  return fmt.Sprintf("%s/content/usr/lib", guestpath.LCOWNvidiaMountPath)
-
 }
 
 // Helper function to find the usr/bin path for the installed nvidia tools.

diff --git a/internal/guest/storage/crypt/crypt.go b/internal/guest/storage/crypt/crypt.go
@@ -164,7 +164,6 @@ func mkfsExt4Command(args []string) error {
 // 4.4. Do a sparse copy of the filesystem into the unencrypted block device.
 // This updates the integrity tags.
 func EncryptDevice(ctx context.Context, source string) (path string, err error) {
-
  uniqueName, err := getUniqueName(source)
  if err != nil {
  return "", errors.Wrapf(err, "failed to generate unique name: %s", source)

diff --git a/internal/guest/storage/devicemapper/devicemapper_test.go b/internal/guest/storage/devicemapper/devicemapper_test.go
@@ -29,6 +29,7 @@ func TestMain(m *testing.M) {
 }
 
 func validateDevice(t *testing.T, p string, sectors int64, writable bool) {
+ t.Helper()
  dev, err := os.OpenFile(p, os.O_RDWR|os.O_SYNC, 0)
  if err != nil {
  t.Fatal(err)
@@ -55,7 +56,6 @@ func validateDevice(t *testing.T, p string, sectors int64, writable bool) {
  } else if !errors.Is(err, unix.EPERM) {
  t.Fatalf("expected EPERM, got %s", err)
  }
-
 }
 
 type device struct {

diff --git a/internal/hcs/errors_test.go b/internal/hcs/errors_test.go
@@ -1,3 +1,5 @@
+//go:build windows
+
 package hcs
 
 import (