Skip to content

Commit

Permalink
lint: fix TestForbiddenImports
Browse files Browse the repository at this point in the history
We need to set `Dir` in the `packages.Load()` call or else the whole
thing doesn't work. It would be really nice if it threw an error instead
of simply doing nothing, but it is what it is. To guard against this in
the future I added an error if the list is empty.

Fix the test then all existing violations, except for `raftlogger` which
will need some extra TLC by the owning team (see cockroachdb#132262)

Closes cockroachdb#132258

Epic: none
Release note: None
  • Loading branch information
rickystewart committed Oct 10, 2024
1 parent afe37da commit 7d94e22
Show file tree
Hide file tree
Showing 14 changed files with 27 additions and 12 deletions.
1 change: 1 addition & 0 deletions pkg/raft/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ go_library(
"//pkg/raft/quorum",
"//pkg/raft/raftpb",
"//pkg/raft/tracker",
"@com_github_cockroachdb_errors//:errors",
],
)

Expand Down
3 changes: 1 addition & 2 deletions pkg/raft/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,8 @@
package raft

import (
"errors"

pb "github.com/cockroachdb/cockroach/pkg/raft/raftpb"
"github.com/cockroachdb/errors"
)

// Bootstrap initializes the RawNode for first use by appending configuration
Expand Down
2 changes: 2 additions & 0 deletions pkg/raft/confchange/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ go_library(
"//pkg/raft/quorum",
"//pkg/raft/raftpb",
"//pkg/raft/tracker",
"@com_github_cockroachdb_errors//:errors",
],
)

Expand All @@ -28,5 +29,6 @@ go_test(
"//pkg/raft/raftpb",
"//pkg/raft/tracker",
"@com_github_cockroachdb_datadriven//:datadriven",
"@com_github_cockroachdb_errors//:errors",
],
)
2 changes: 1 addition & 1 deletion pkg/raft/confchange/confchange.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@
package confchange

import (
"errors"
"fmt"
"strings"

"github.com/cockroachdb/cockroach/pkg/raft/quorum"
pb "github.com/cockroachdb/cockroach/pkg/raft/raftpb"
"github.com/cockroachdb/cockroach/pkg/raft/tracker"
"github.com/cockroachdb/errors"
)

// Changer facilitates configuration changes. It exposes methods to handle
Expand Down
2 changes: 1 addition & 1 deletion pkg/raft/confchange/datadriven_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
package confchange

import (
"errors"
"fmt"
"strconv"
"strings"
Expand All @@ -27,6 +26,7 @@ import (
pb "github.com/cockroachdb/cockroach/pkg/raft/raftpb"
"github.com/cockroachdb/cockroach/pkg/raft/tracker"
"github.com/cockroachdb/datadriven"
"github.com/cockroachdb/errors"
)

func TestConfChangeDataDriven(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/raft/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ package raft

import (
"context"
"errors"

pb "github.com/cockroachdb/cockroach/pkg/raft/raftpb"
"github.com/cockroachdb/errors"
)

type SnapshotStatus int
Expand Down
2 changes: 1 addition & 1 deletion pkg/raft/raft.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ package raft
import (
"bytes"
"crypto/rand"
"errors"
"fmt"
"math"
"math/big"
Expand All @@ -32,6 +31,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/raft/quorum"
pb "github.com/cockroachdb/cockroach/pkg/raft/raftpb"
"github.com/cockroachdb/cockroach/pkg/raft/tracker"
"github.com/cockroachdb/errors"
)

const (
Expand Down
1 change: 1 addition & 0 deletions pkg/raft/rafttest/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ go_library(
"//pkg/raft/raftpb",
"//pkg/raft/tracker",
"@com_github_cockroachdb_datadriven//:datadriven",
"@com_github_cockroachdb_errors//:errors",
"@com_github_stretchr_testify//require",
],
)
Expand Down
2 changes: 1 addition & 1 deletion pkg/raft/rafttest/interaction_env_handler_add_nodes.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,14 @@
package rafttest

import (
"errors"
"fmt"
"reflect"
"testing"

"github.com/cockroachdb/cockroach/pkg/raft"
pb "github.com/cockroachdb/cockroach/pkg/raft/raftpb"
"github.com/cockroachdb/datadriven"
"github.com/cockroachdb/errors"
)

func (env *InteractionEnv) handleAddNodes(t *testing.T, d datadriven.TestData) error {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@
package rafttest

import (
"errors"
"fmt"
"testing"

"github.com/cockroachdb/cockroach/pkg/raft"
"github.com/cockroachdb/cockroach/pkg/raft/raftpb"
"github.com/cockroachdb/datadriven"
"github.com/cockroachdb/errors"
)

func (env *InteractionEnv) handleProcessAppendThread(t *testing.T, d datadriven.TestData) error {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@
package rafttest

import (
"errors"
"testing"

"github.com/cockroachdb/datadriven"
"github.com/cockroachdb/errors"
)

func (env *InteractionEnv) handleReportUnreachable(t *testing.T, d datadriven.TestData) error {
Expand Down
3 changes: 1 addition & 2 deletions pkg/raft/rawnode.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,9 @@
package raft

import (
"errors"

pb "github.com/cockroachdb/cockroach/pkg/raft/raftpb"
"github.com/cockroachdb/cockroach/pkg/raft/tracker"
"github.com/cockroachdb/errors"
)

// ErrStepLocalMsg is returned when try to step a local raft message
Expand Down
2 changes: 1 addition & 1 deletion pkg/raft/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@
package raft

import (
"errors"
"sync"

pb "github.com/cockroachdb/cockroach/pkg/raft/raftpb"
"github.com/cockroachdb/errors"
)

// ErrCompacted is returned by Storage.Entries/Compact when a requested
Expand Down
13 changes: 13 additions & 0 deletions pkg/testutils/lint/lint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1776,12 +1776,19 @@ func TestLint(t *testing.T) {
pkgs, err := packages.Load(
&packages.Config{
Mode: packages.NeedImports | packages.NeedName,
Dir: crdbDir,
},
pkgPath,
)
if err != nil {
return errors.Wrapf(err, "error loading package %s", pkgPath)
}
// NB: if no packages were found, this API confusingly
// returns no error, so we need to explicitly check that
// something was returned.
if len(pkgs) == 0 {
return errors.Newf("could not list packages under %s", pkgPath)
}
for _, pkg := range pkgs {
for _, s := range pkg.Imports {
arg.Out <- pkg.PkgPath + ": " + s.PkgPath
Expand All @@ -1801,6 +1808,7 @@ func TestLint(t *testing.T) {
stream.GrepNot(`cockroach/pkg/util/sysutil: syscall$`),
stream.GrepNot(`cockroachdb/cockroach/pkg/build/bazel/util/tinystringer: errors$`),
stream.GrepNot(`cockroachdb/cockroach/pkg/build/engflow: github\.com/golang/protobuf/proto$`),
stream.GrepNot(`cockroachdb/cockroach/pkg/build/engflow: log$`),
stream.GrepNot(`cockroachdb/cockroach/pkg/util/grpcutil: github\.com/cockroachdb\/errors\/errbase$`),
stream.GrepNot(`cockroachdb/cockroach/pkg/util/future: github\.com/cockroachdb\/errors\/errbase$`),
stream.GrepNot(`cockroach/pkg/roachprod/install: syscall$`), // TODO: switch to sysutil
Expand All @@ -1814,6 +1822,11 @@ func TestLint(t *testing.T) {
stream.GrepNot(`cockroachdb/cockroach/pkg/kv/kvpb/gen: log$`),
stream.GrepNot(`cockroachdb/cockroach/pkg/util/log/gen: log$`),
stream.GrepNot(`cockroach/pkg/util/uuid: github\.com/satori/go\.uuid$`),
// See #132262.
stream.GrepNot(`github.com/cockroachdb/cockroach/pkg/raft: log$`),
stream.GrepNot(`github.com/cockroachdb/cockroach/pkg/raft/raftlogger: log$`),
stream.GrepNot(`github.com/cockroachdb/cockroach/pkg/raft/rafttest: log$`),
stream.GrepNot(`github.com/cockroachdb/cockroach/pkg/workload/debug: log$`),
), func(s string) {
pkgStr := strings.Split(s, ": ")
importingPkg, importedPkg := pkgStr[0], pkgStr[1]
Expand Down

0 comments on commit 7d94e22

Please sign in to comment.