use panic

Author: stephanos

common/testing/testhooks/noop_impl.go

@@ -3,46 +3,38 @@
 package testhooks
 
 import (
-	"go.temporal.io/server/common/log"
-	"go.temporal.io/server/common/softassert"
 	"go.uber.org/fx"
 )
 
 var Module = fx.Options(
 	fx.Provide(NewTestHooks),
 )
 
-// TestHooks (in production mode) is an empty struct just so the build works.
-// See TestHooks in test_impl.go.
-//
-// TestHooks are an inherently unclean way of writing tests. They require mixing test-only
-// concerns into production code. In general you should prefer other ways of writing tests
-// wherever possible, and only use TestHooks sparingly, as a last resort.
-type TestHooks struct {
-	logger log.Logger
-}
+// noopTestHooks is a noop implementation of TestHooks for production builds.
+type noopTestHooks struct{}
 
-func NewTestHooks(logger log.Logger) TestHooks {
-	return TestHooks{logger: logger}
+func NewTestHooks() TestHooks {
+	return noopTestHooks{}
 }
 
+func (noopTestHooks) testHooks() {}
+
 // Get gets the value of a test hook. In production mode it always returns the zero value and
 // false, which hopefully the compiler will inline and remove the hook as dead code.
 //
 // TestHooks should be used very sparingly, see comment on TestHooks.
-func Get[T any](_ TestHooks, key Key) (T, bool) {
+func Get[T any](_ TestHooks, _ Key) (T, bool) {
 	var zero T
 	return zero, false
 }
 
 // Call calls a func() hook if present.
 //
 // TestHooks should be used very sparingly, see comment on TestHooks.
-func Call(_ TestHooks, key Key) {
+func Call(_ TestHooks, _ Key) {
 }
 
 // Set is only to be used by test code together with the test_dep build tag.
-func Set[T any](th TestHooks, key Key, val T) func() {
-	softassert.Fail(th.logger, "testhooks.Set called without test_dep build tag")
-	return func() {}
+func Set[T any](_ TestHooks, _ Key, _ T) func() {
+	panic("testhooks.Set called but TestHooks are not enabled: use -tags=test_dep when running `go test`")
 }

common/testing/testhooks/test_impl.go

@@ -5,38 +5,24 @@ package testhooks
 import (
 	"sync"
 
-	"go.temporal.io/server/common/log"
 	"go.uber.org/fx"
 )
 
 var Module = fx.Options(
 	fx.Provide(NewTestHooks),
 )
 
-type (
-	// TestHooks holds a registry of active test hooks. It should be obtained through fx and
-	// used with Get and Set.
-	//
-	// TestHooks are an inherently unclean way of writing tests. They require mixing test-only
-	// concerns into production code. In general you should prefer other ways of writing tests
-	// wherever possible, and only use TestHooks sparingly, as a last resort.
-	TestHooks interface {
-		// private accessors; access must go through package-level Get/Set
-		get(Key) (any, bool)
-		set(Key, any)
-		del(Key)
-	}
-
-	// testHooksImpl is an implementation of TestHooks.
-	testHooksImpl struct {
-		m sync.Map
-	}
-)
+// testHooksImpl is an implementation of TestHooks.
+type testHooksImpl struct {
+	m sync.Map
+}
 
-func NewTestHooks(_ log.Logger) TestHooks {
+func NewTestHooks() TestHooks {
 	return &testHooksImpl{}
 }
 
+func (*testHooksImpl) testHooks() {}
+
 // Get gets the value of a test hook from the registry.
 //
 // TestHooks should be used sparingly, see comment on TestHooks.
@@ -45,9 +31,13 @@ func Get[T any](th TestHooks, key Key) (T, bool) {
 	if th == nil {
 		return zero, false
 	}
-	if val, ok := th.get(key); ok {
+	impl, ok := th.(*testHooksImpl)
+	if !ok {
+		return zero, false
+	}
+	if val, ok := impl.m.Load(key); ok {
 		// this is only used in test so we want to panic on type mismatch:
-		return val.(T), ok // nolint:revive
+		return val.(T), ok //nolint:revive
 	}
 	return zero, false
 }
@@ -64,18 +54,7 @@ func Call(th TestHooks, key Key) {
 // Set sets a test hook to a value and returns a cleanup function to unset it.
 // Calls to Set and the cleanup function should form a stack.
 func Set[T any](th TestHooks, key Key, val T) func() {
-	th.set(key, val)
-	return func() { th.del(key) }
-}
-
-func (th *testHooksImpl) get(key Key) (any, bool) {
-	return th.m.Load(key)
-}
-
-func (th *testHooksImpl) set(key Key, val any) {
-	th.m.Store(key, val)
-}
-
-func (th *testHooksImpl) del(key Key) {
-	th.m.Delete(key)
+	impl := th.(*testHooksImpl)
+	impl.m.Store(key, val)
+	return func() { impl.m.Delete(key) }
 }

tests/testcore/onebox.go

@@ -214,7 +214,7 @@ func newTemporal(t *testing.T, params *TemporalParams) *TemporalImpl {
 		tlsConfigProvider:                params.TLSConfigProvider,
 		captureMetricsHandler:            params.CaptureMetricsHandler,
 		dcClient:                         dynamicconfig.NewMemoryClient(),
-		testHooks:                        testhooks.NewTestHooks(params.Logger),
+		testHooks:                        testhooks.NewTestHooks(),
 		serviceFxOptions:                 params.ServiceFxOptions,
 		taskCategoryRegistry:             params.TaskCategoryRegistry,
 		hostsByProtocolByService:         params.HostsByProtocolByService,