lint: Disallow use of "normal" contexts

Our testhelper context has recently gained the ability to verify that
feature flags are exercised as expected. This is an important safety
guard which can help us to avoid releasing untested and thus potentially
buggy code.

Disallow usage of "normal" contexts which don't have this ability such
that we do not introduce their usage by accident.

7 files changed, 48 insertions, 23 deletions

diff --git a/.golangci.yml b/.golangci.yml
index dce1c42d7..b1359a125 100644
--- a/.golangci.yml
+++ b/.golangci.yml
@@ -53,6 +53,11 @@ linters-settings:
       # following functions is thus disallowed. and a code smell.
       - ^context.WithDeadline$
       - ^context.WithTimeout$
+      # Tests should always use `testhelper.Context()`: this context has
+      # special handling for feature flags which allows us to assert that
+      # they're tested as expected.
+      - ^context.Background$
+      - ^context.TODO$
   stylecheck:
     # ST1000 checks for missing package comments. We don't use these for most
     # packages, so let's disable this check.
diff --git a/internal/gitaly/storage/servers_test.go b/internal/gitaly/storage/servers_test.go
index 5c6def8da..f10666828 100644
--- a/internal/gitaly/storage/servers_test.go
+++ b/internal/gitaly/storage/servers_test.go
@@ -81,6 +81,8 @@ func TestInjectGitalyServers(t *testing.T) {
 	}
 
 	t.Run("brand new context", func(t *testing.T) {
+		//nolint:forbidigo // We need to check for metadata and thus cannot use the
+		// testhelper context, which injects feature flags.
 		ctx := context.Background()
 
 		check(t, ctx)
@@ -89,6 +91,8 @@ func TestInjectGitalyServers(t *testing.T) {
 	t.Run("context with existing outgoing metadata should not be re-written", func(t *testing.T) {
 		existing := metadata.New(map[string]string{"foo": "bar"})
 
+		//nolint:forbidigo // We need to check for metadata and thus cannot use the
+		// testhelper context, which injects feature flags.
 		ctx := metadata.NewOutgoingContext(context.Background(), existing)
 		check(t, ctx)
 
diff --git a/internal/log/log_test.go b/internal/log/log_test.go
index 4c508e057..93f4b1325 100644
--- a/internal/log/log_test.go
+++ b/internal/log/log_test.go
@@ -27,7 +27,7 @@ import (
 )
 
 func TestPayloadBytes(t *testing.T) {
-	ctx := context.Background()
+	ctx := createContext()
 
 	logger, hook := test.NewNullLogger()
 
@@ -261,7 +261,7 @@ func TestConfigure(t *testing.T) {
 func TestMessageProducer(t *testing.T) {
 	triggered := false
 	MessageProducer(func(ctx context.Context, format string, level logrus.Level, code codes.Code, err error, fields logrus.Fields) {
-		require.Equal(t, context.Background(), ctx)
+		require.Equal(t, createContext(), ctx)
 		require.Equal(t, "format-stub", format)
 		require.Equal(t, logrus.DebugLevel, level)
 		require.Equal(t, codes.OutOfRange, code)
@@ -272,20 +272,20 @@ func TestMessageProducer(t *testing.T) {
 		return logrus.Fields{"a": 1}
 	}, func(context.Context) logrus.Fields {
 		return logrus.Fields{"b": "test"}
-	})(context.Background(), "format-stub", logrus.DebugLevel, codes.OutOfRange, assert.AnError, logrus.Fields{"c": "stub"})
+	})(createContext(), "format-stub", logrus.DebugLevel, codes.OutOfRange, assert.AnError, logrus.Fields{"c": "stub"})
 	require.True(t, triggered)
 }
 
 func TestPropagationMessageProducer(t *testing.T) {
 	t.Run("empty context", func(t *testing.T) {
-		ctx := context.Background()
+		ctx := createContext()
 		mp := PropagationMessageProducer(func(context.Context, string, logrus.Level, codes.Code, error, logrus.Fields) {})
 		mp(ctx, "", logrus.DebugLevel, codes.OK, nil, nil)
 	})
 
 	t.Run("context with holder", func(t *testing.T) {
 		holder := new(messageProducerHolder)
-		ctx := context.WithValue(context.Background(), messageProducerHolderKey{}, holder)
+		ctx := context.WithValue(createContext(), messageProducerHolderKey{}, holder)
 		triggered := false
 		mp := PropagationMessageProducer(func(ctx context.Context, format string, level logrus.Level, code codes.Code, err error, fields logrus.Fields) {
 			triggered = true
@@ -313,7 +313,7 @@ func TestPerRPCLogHandler(t *testing.T) {
 	}
 
 	t.Run("check propagation", func(t *testing.T) {
-		ctx := context.Background()
+		ctx := createContext()
 		ctx = lh.TagConn(ctx, &stats.ConnTagInfo{})
 		lh.HandleConn(ctx, &stats.ConnBegin{})
 		ctx = lh.TagRPC(ctx, &stats.RPCTagInfo{})
@@ -334,7 +334,7 @@ func TestPerRPCLogHandler(t *testing.T) {
 	})
 
 	t.Run("log handling", func(t *testing.T) {
-		ctx := ctxlogrus.ToContext(context.Background(), logrus.NewEntry(logrus.New()))
+		ctx := ctxlogrus.ToContext(createContext(), logrus.NewEntry(logrus.New()))
 		ctx = lh.TagRPC(ctx, &stats.RPCTagInfo{})
 		mpp := ctx.Value(messageProducerHolderKey{}).(*messageProducerHolder)
 		mpp.format = "message"
@@ -381,7 +381,7 @@ func TestUnaryLogDataCatcherServerInterceptor(t *testing.T) {
 
 	t.Run("propagates call", func(t *testing.T) {
 		interceptor := UnaryLogDataCatcherServerInterceptor()
-		resp, err := interceptor(context.Background(), nil, nil, func(ctx context.Context, req interface{}) (interface{}, error) {
+		resp, err := interceptor(createContext(), nil, nil, func(ctx context.Context, req interface{}) (interface{}, error) {
 			return 42, assert.AnError
 		})
 
@@ -391,7 +391,7 @@ func TestUnaryLogDataCatcherServerInterceptor(t *testing.T) {
 
 	t.Run("no logger", func(t *testing.T) {
 		mpp := &messageProducerHolder{}
-		ctx := context.WithValue(context.Background(), messageProducerHolderKey{}, mpp)
+		ctx := context.WithValue(createContext(), messageProducerHolderKey{}, mpp)
 
 		interceptor := UnaryLogDataCatcherServerInterceptor()
 		_, _ = interceptor(ctx, nil, nil, handlerStub)
@@ -400,7 +400,7 @@ func TestUnaryLogDataCatcherServerInterceptor(t *testing.T) {
 
 	t.Run("caught", func(t *testing.T) {
 		mpp := &messageProducerHolder{}
-		ctx := context.WithValue(context.Background(), messageProducerHolderKey{}, mpp)
+		ctx := context.WithValue(createContext(), messageProducerHolderKey{}, mpp)
 		ctx = ctxlogrus.ToContext(ctx, logrus.New().WithField("a", 1))
 		interceptor := UnaryLogDataCatcherServerInterceptor()
 		_, _ = interceptor(ctx, nil, nil, handlerStub)
@@ -411,7 +411,7 @@ func TestUnaryLogDataCatcherServerInterceptor(t *testing.T) {
 func TestStreamLogDataCatcherServerInterceptor(t *testing.T) {
 	t.Run("propagates call", func(t *testing.T) {
 		interceptor := StreamLogDataCatcherServerInterceptor()
-		ss := &grpcmw.WrappedServerStream{WrappedContext: context.Background()}
+		ss := &grpcmw.WrappedServerStream{WrappedContext: createContext()}
 		err := interceptor(nil, ss, nil, func(interface{}, grpc.ServerStream) error {
 			return assert.AnError
 		})
@@ -421,7 +421,7 @@ func TestStreamLogDataCatcherServerInterceptor(t *testing.T) {
 
 	t.Run("no logger", func(t *testing.T) {
 		mpp := &messageProducerHolder{}
-		ctx := context.WithValue(context.Background(), messageProducerHolderKey{}, mpp)
+		ctx := context.WithValue(createContext(), messageProducerHolderKey{}, mpp)
 
 		interceptor := StreamLogDataCatcherServerInterceptor()
 		ss := &grpcmw.WrappedServerStream{WrappedContext: ctx}
@@ -430,7 +430,7 @@ func TestStreamLogDataCatcherServerInterceptor(t *testing.T) {
 
 	t.Run("caught", func(t *testing.T) {
 		mpp := &messageProducerHolder{}
-		ctx := context.WithValue(context.Background(), messageProducerHolderKey{}, mpp)
+		ctx := context.WithValue(createContext(), messageProducerHolderKey{}, mpp)
 		ctx = ctxlogrus.ToContext(ctx, logrus.New().WithField("a", 1))
 
 		interceptor := StreamLogDataCatcherServerInterceptor()
@@ -439,3 +439,9 @@ func TestStreamLogDataCatcherServerInterceptor(t *testing.T) {
 		assert.Equal(t, logrus.Fields{"a": 1}, mpp.fields)
 	})
 }
+
+//nolint:forbidigo // We cannot use `testhelper.Context()` because of a cyclic dependency between
+// this package and the `testhelper` package.
+func createContext() context.Context {
+	return context.Background()
+}
diff --git a/internal/metadata/featureflag/context_test.go b/internal/metadata/featureflag/context_test.go
index b938aa06f..998c0f259 100644
--- a/internal/metadata/featureflag/context_test.go
+++ b/internal/metadata/featureflag/context_test.go
@@ -14,8 +14,14 @@ var (
 	ffB = FeatureFlag{"feature-b", false}
 )
 
+//nolint:forbidigo // We cannot use `testhelper.Context()` given that it would inject feature flags
+// already.
+func createContext() context.Context {
+	return context.Background()
+}
+
 func TestIncomingCtxWithFeatureFlag(t *testing.T) {
-	ctx := context.Background()
+	ctx := createContext()
 	require.False(t, ffA.IsEnabled(ctx))
 	require.False(t, ffB.IsEnabled(ctx))
 
@@ -42,7 +48,7 @@ func TestIncomingCtxWithFeatureFlag(t *testing.T) {
 }
 
 func TestOutgoingCtxWithFeatureFlag(t *testing.T) {
-	ctx := context.Background()
+	ctx := createContext()
 	require.False(t, ffA.IsEnabled(ctx))
 	require.False(t, ffB.IsEnabled(ctx))
 
@@ -56,7 +62,7 @@ func TestOutgoingCtxWithFeatureFlag(t *testing.T) {
 		require.True(t, ok)
 
 		// It should be enabled after converting it to an incoming context though.
-		ctx = metadata.NewIncomingContext(context.Background(), md)
+		ctx = metadata.NewIncomingContext(createContext(), md)
 		require.True(t, ffA.IsEnabled(ctx))
 	})
 
@@ -67,7 +73,7 @@ func TestOutgoingCtxWithFeatureFlag(t *testing.T) {
 		md, ok := metadata.FromOutgoingContext(ctx)
 		require.True(t, ok)
 
-		ctx = metadata.NewIncomingContext(context.Background(), md)
+		ctx = metadata.NewIncomingContext(createContext(), md)
 		require.False(t, ffA.IsEnabled(ctx))
 	})
 
@@ -106,7 +112,7 @@ func TestGRPCMetadataFeatureFlag(t *testing.T) {
 	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
 			md := metadata.New(tc.headers)
-			ctx := metadata.NewIncomingContext(context.Background(), md)
+			ctx := metadata.NewIncomingContext(createContext(), md)
 
 			require.Equal(t, tc.enabled, FeatureFlag{tc.flag, tc.onByDefault}.IsEnabled(ctx))
 		})
@@ -121,7 +127,7 @@ func TestAllEnabledFlags(t *testing.T) {
 		ffPrefix + "bar":  "TRUE",  // not enabled
 	}
 
-	ctx := metadata.NewIncomingContext(context.Background(), metadata.New(flags))
+	ctx := metadata.NewIncomingContext(createContext(), metadata.New(flags))
 	require.ElementsMatch(t, AllFlags(ctx), []string{"meow:true", "foo:true", "woof:false", "bar:TRUE"})
 }
 
@@ -135,7 +141,7 @@ func TestRaw(t *testing.T) {
 	}
 
 	t.Run("RawFromContext", func(t *testing.T) {
-		ctx := context.Background()
+		ctx := createContext()
 		ctx = IncomingCtxWithFeatureFlag(ctx, enabledFlag, true)
 		ctx = IncomingCtxWithFeatureFlag(ctx, disabledFlag, false)
 
@@ -143,7 +149,7 @@ func TestRaw(t *testing.T) {
 	})
 
 	t.Run("OutgoingWithRaw", func(t *testing.T) {
-		outgoingMD, ok := metadata.FromOutgoingContext(OutgoingWithRaw(context.Background(), raw))
+		outgoingMD, ok := metadata.FromOutgoingContext(OutgoingWithRaw(createContext(), raw))
 		require.True(t, ok)
 		require.Equal(t, metadata.MD{
 			ffPrefix + enabledFlag.Name:  {"true"},
diff --git a/internal/metadata/featureflag/featureflag_test.go b/internal/metadata/featureflag/featureflag_test.go
index ef9dc52ca..8944e21fd 100644
--- a/internal/metadata/featureflag/featureflag_test.go
+++ b/internal/metadata/featureflag/featureflag_test.go
@@ -1,7 +1,6 @@
 package featureflag
 
 import (
-	"context"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -74,7 +73,7 @@ func TestFeatureFlag_enabled(t *testing.T) {
 		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
-			ctx := metadata.NewIncomingContext(context.Background(), metadata.New(tc.headers))
+			ctx := metadata.NewIncomingContext(createContext(), metadata.New(tc.headers))
 
 			ff := FeatureFlag{tc.flag, tc.onByDefault}
 			require.Equal(t, tc.enabled, ff.IsEnabled(ctx))
diff --git a/internal/middleware/featureflag/featureflag_handler_test.go b/internal/middleware/featureflag/featureflag_handler_test.go
index be0ccfb43..f3b68b017 100644
--- a/internal/middleware/featureflag/featureflag_handler_test.go
+++ b/internal/middleware/featureflag/featureflag_handler_test.go
@@ -71,6 +71,8 @@ func setup() (context.Context, *test.Hook) {
 }
 
 func setupContext() (context.Context, *test.Hook) {
+	//nolint:forbidigo // We don't want to set up the feature flags which `testhelper.Context()`
+	// would inject here.
 	ctx := context.Background()
 	logger := logrus.New()
 	logger.SetOutput(io.Discard)
diff --git a/internal/praefect/coordinator_test.go b/internal/praefect/coordinator_test.go
index 02bb0c4a2..70dc062aa 100644
--- a/internal/praefect/coordinator_test.go
+++ b/internal/praefect/coordinator_test.go
@@ -2213,6 +2213,9 @@ func TestStreamParametersContext(t *testing.T) {
 		return metadata.Pairs(pairs...)
 	}
 
+	//nolint:forbidigo // We explicitly test context values, so we cannot use the testhelper
+	// context here given that it would contain unrelated data and thus change the system under
+	// test.
 	for _, tc := range []struct {
 		desc               string
 		setupContext       func() context.Context