Skip to content

Commit

Permalink
Unrelated: Fix nolint ping-pong
Browse files Browse the repository at this point in the history
Nolintlint wants the //nolint directives without a leading space
GoFmt wants them with a leading space, ...
    except if nolint has no space afterwards:

// nolint:foo // <-- malformed
//nolint :foo // <-- malformed
//nolint: foo // <-- malformed

//nolint:foo // <-- OK

See golangci/golangci-lint#3110 (comment)
  • Loading branch information
EduardGomezEscandell committed Mar 20, 2023
1 parent 46f91c2 commit d3ea0b7
Show file tree
Hide file tree
Showing 11 changed files with 36 additions and 20 deletions.
9 changes: 6 additions & 3 deletions windows-agent/internal/distros/database/database_test.go
Expand Up @@ -33,8 +33,9 @@ const (
badDbFileContents
)

//nolint: tparallel
// Subtests are parallel but the test itself is not due to the calls to RegisterDistro.
//
//nolint:tparallel
func TestNew(t *testing.T) {
ctx := context.Background()
if wsl.MockAvailable() {
Expand Down Expand Up @@ -139,8 +140,9 @@ func TestDatabaseGetAll(t *testing.T) {
}
}

//nolint: tparallel
// Subtests are parallel but the test itself is not due to the calls to RegisterDistro.
//
//nolint:tparallel
func TestDatabaseGet(t *testing.T) {
ctx := context.Background()
if wsl.MockAvailable() {
Expand Down Expand Up @@ -195,8 +197,9 @@ func TestDatabaseGet(t *testing.T) {
}
}

//nolint: tparallel
// Subtests are parallel but the test itself is not due to the calls to RegisterDistro.
//
//nolint:tparallel
func TestDatabaseDump(t *testing.T) {
ctx := context.Background()
if wsl.MockAvailable() {
Expand Down
3 changes: 2 additions & 1 deletion windows-agent/internal/distros/database/export_test.go
Expand Up @@ -15,8 +15,9 @@ func (in SerializableDistro) NewDistro(ctx context.Context, storageDir string) (

// NewSerializableDistro is a wrapper around newSerializableDistro so as to make it accessible to tests.
//
// nolint: revive
// unexported-return false positive! SerializableDistro is exported, even if it is an alias to an unexported type.
//
//nolint:revive
func NewSerializableDistro(d *distro.Distro) SerializableDistro {
return newSerializableDistro(d)
}
Expand Down
Expand Up @@ -70,8 +70,9 @@ func TestSerializableDistroMarshallUnmarshall(t *testing.T) {
}
}

//nolint: tparallel
// Subtests are parallel but the test itself is not due to the calls to RegisterDistro.
//
//nolint:tparallel
func TestSerializableDistroNewDistro(t *testing.T) {
ctx := context.Background()
if wsl.MockAvailable() {
Expand Down
6 changes: 4 additions & 2 deletions windows-agent/internal/distros/distro/distro_test.go
Expand Up @@ -246,8 +246,9 @@ func TestKeepAwake(t *testing.T) {
}
}

// nolint: tparallel
// Subtests are parallel but the test itself is not due to the calls to RegisterDistro.
//
//nolint:tparallel
func TestWorkerConstruction(t *testing.T) {
ctx := context.Background()
if wsl.MockAvailable() {
Expand Down Expand Up @@ -336,8 +337,9 @@ func TestInvalidateIdempotent(t *testing.T) {
require.False(t, (*w).stopCalled, "worker Stop should not be called in subsequent invalidations")
}

// nolint: tparallel
// Subtests are parallel but the test itself is not due to the calls to RegisterDistro.
//
//nolint:tparallel
func TestWorkerWrappers(t *testing.T) {
ctx := context.Background()
if wsl.MockAvailable() {
Expand Down
3 changes: 2 additions & 1 deletion windows-agent/internal/distros/distro/export_test.go
Expand Up @@ -29,8 +29,9 @@ type Identity = identity

// GetIdentity returns a reference to the distro's identity.
//
// nolint: revive
// False positive, Identity is exported.
//
//nolint:revive
func (d *Distro) GetIdentity() *Identity {
return &d.identity
}
9 changes: 6 additions & 3 deletions windows-agent/internal/distros/task/task_test.go
Expand Up @@ -35,8 +35,9 @@ func TestRegistry(t *testing.T) {
require.ElementsMatch(t, want, got, "registry should contain only the registered tasks")
}

//nolint: tparallel
// Cannot make test parallel because of BackupRegistry.
//
//nolint:tparallel
func TestMarshal(t *testing.T) {
task.BackupRegistry(t)
task.Register[testTask]()
Expand Down Expand Up @@ -69,8 +70,9 @@ func TestMarshal(t *testing.T) {
}
}

//nolint: tparallel
// Cannot make test parallel because of BackupRegistry.
//
//nolint:tparallel
func TestUnmarshal(t *testing.T) {
task.BackupRegistry(t)
task.Register[testTask]()
Expand Down Expand Up @@ -116,8 +118,9 @@ func TestUnmarshal(t *testing.T) {
}
}

//nolint: tparallel
// Cannot make test parallel because of BackupRegistry.
//
//nolint:tparallel
func TestMarsallUnmarshall(t *testing.T) {
task.BackupRegistry(t)
task.Register[testTask]()
Expand Down
Expand Up @@ -66,7 +66,7 @@ func (s *Service) Connected(stream agentapi.WSLInstance_ConnectedServer) error {
return fmt.Errorf("connection from %q: %v", info.WslName, err)
}

//nolint: errcheck // We don't care about this error because we're cleaning up
//nolint:errcheck // We don't care about this error because we're cleaning up
defer d.SetConnection(nil)

log.Debugf(context.TODO(), "Connection to Linux-side service established")
Expand Down
15 changes: 10 additions & 5 deletions windows-agent/internal/proservices/wslinstance/wslinstance_test.go
Expand Up @@ -78,8 +78,9 @@ func (w step) String() string {
return fmt.Sprintf("Unknown when (%d)", int(w))
}

// nolint: tparallel
// Subtests are parallel but the test itself is not due to the calls to RegisterDistro.
//
//nolint:tparallel
func TestConnected(t *testing.T) {
ctx := context.Background()
if wsl.MockAvailable() {
Expand Down Expand Up @@ -245,9 +246,10 @@ func TestConnected(t *testing.T) {
// testLoggerInterceptor replaces the logging middleware by printing the return
// error of Connected to the test Log.
//
// nolint: thelper
// The logs would be reported to come from the entrails of the GRPC module. It's more helpful to reference this function to
// see that it is the middleware reporting.
//
//nolint:thelper
func testLoggerInterceptor(t *testing.T) grpc.StreamServerInterceptor {
return func(srv interface{}, stream grpc.ServerStream, info *grpc.StreamServerInfo, handler grpc.StreamHandler) error {
if err := handler(srv, stream); err != nil {
Expand Down Expand Up @@ -286,9 +288,10 @@ func (s *wrappedService) Connected(stream agentapi.WSLInstance_ConnectedServer)
// - if ok is true, returnErr is the return value of Connected.
// - if ok is false, the wait times out hence Connected has not returned yet. returnedErr is therefore not valid.
//
// nolint: revive
// Returning the error as first argument is strange but it makes sense here, we mimic the
// (value, ok) return type of a map access.
//
//nolint:revive
func (s *wrappedService) wait(timeout time.Duration) (returnedErr error, connectedHasReturned bool) {
select {
case returnedErr = <-s.Errch:
Expand All @@ -298,8 +301,9 @@ func (s *wrappedService) wait(timeout time.Duration) (returnedErr error, connect
}
}

// nolint: revive
// testing.T should go before context, I won't listen to anyone arguing the contrary.
//
//nolint:revive
func serveWSLInstance(t *testing.T, ctx context.Context, srv wrappedService) (server *grpc.Server, address string) {
t.Helper()

Expand Down Expand Up @@ -327,8 +331,9 @@ type wslDistroMock struct {

// newWslDistroMock creates a wslDistroMock, establishing a connection to the control stream.
//
// nolint: revive
// testing.T should go before context, regardless of what these linters say.
//
//nolint:revive
func newWslDistroMock(t *testing.T, ctx context.Context, ctrlAddr string) (mock *wslDistroMock) {
t.Helper()

Expand Down
2 changes: 1 addition & 1 deletion windows-agent/internal/testutils/wsl.go
Expand Up @@ -37,7 +37,7 @@ func RandomDistroName(t *testing.T) (name string) {

testFullNormalized := normalizeName(t, strings.ReplaceAll(t.Name(), "/", "--"))

//nolint: gosec // No need to be cryptographically secure
//nolint:gosec // No need to be cryptographically secure
return fmt.Sprintf("%s_%s_%d", testDistroPrefix, testFullNormalized, rand.Uint64())
}

Expand Down
2 changes: 1 addition & 1 deletion windows-agent/internal/testutils/wsl_windows.go
Expand Up @@ -58,7 +58,7 @@ func powershellOutputf(t *testing.T, command string, args ...any) string {

cmd := fmt.Sprintf(command, args...)

//nolint: gosec // This function is only used in tests so no arbitrary code execution here
//nolint:gosec // This function is only used in tests so no arbitrary code execution here
out, err := exec.Command("powershell", "-Command", cmd).CombinedOutput()
require.NoError(t, err, "Non-zero return code for command:\n%s\nOutput:%s", cmd, out)

Expand Down
2 changes: 1 addition & 1 deletion wsl-pro-service/internal/systeminfo/systeminfo.go
Expand Up @@ -47,7 +47,7 @@ func fillOsRelease(info *agentapi.DistroInfo) error {
}

var marshaller struct {
//nolint: revive
//nolint:revive
// ini mapper is strict with naming, so we cannot rename Id -> ID as the linter suggests
Id, VersionId, PrettyName string
}
Expand Down

0 comments on commit d3ea0b7

Please sign in to comment.