Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

testsuite.TestUpdateCallback method should be given no-ops when not provided #1860

Open
mnichols opened this issue Mar 10, 2025 · 0 comments
Labels
bug Something isn't working

Comments

@mnichols
Copy link

Expected Behavior

I should be able to just provide a callback for OnComplete in the testsuite.TestUpdateCallbacks.
Either that, or I should get an error that tells me what is really wrong (not nil pointer).

Actual Behavior

This doesn't seem to check that each callback is provided so will panic when invoked downstream. These nil pointer exceptions are super hard to understand coming out of this test suite and make testing brittle with UpdateWorkflow.

I recommend we either:

  1. catch the error and provided a more meaningful error
  2. (better) provide no-op stubs when a particular callback is not provided (eg OnAccept: func() {})

Somewhere in here probably:
https://github.com/temporalio/sdk-go/blob/master/internal/internal_workflow_testsuite.go#L2986

Steps to Reproduce the Problem

This will fail because I don't have OnAccept provided as a noop.
IMO this should be an opt-in facility.

package myworkflow

import (
	"fmt"
	"github.com/google/uuid"
	"github.com/stretchr/testify/assert"
	"github.com/stretchr/testify/require"
	"go.temporal.io/sdk/testsuite"
	"go.temporal.io/sdk/workflow"
	"testing"
)

func MyWorkflow(ctx workflow.Context) error {

	if err := workflow.SetUpdateHandler(
		ctx,
		"add",
		func(ctx workflow.Context) (int, error) {
			fmt.Printf("UpdateHandler called")
			return 42, nil
		}); err != nil {

		return fmt.Errorf("could not set update: %w", err)
	}
	return nil
}

func TestUpdateReturnsAppropriateResponse(t *testing.T) {
	A := assert.New(t)
	R := require.New(t)

	s := &testsuite.WorkflowTestSuite{}

	env := s.NewTestWorkflowEnvironment()

	env.RegisterWorkflow(StandaloneScreening)
	env.RegisterDelayedCallback(func() {
		env.UpdateWorkflow("add", uuid.New().String(), &testsuite.TestUpdateCallback{
			//  uncomment this callback to let it pass
                         // OnAccept: func() {},
			OnReject: func(err error) {
				R.NoError(err)
			},
			OnComplete: func(i interface{}, err error) {
				fmt.Printf("OnComplete: %v\n", i)
			},
		})
	}, 0)
	env.ExecuteWorkflow(MyWorkflow)
	err := env.GetWorkflowError()
	R.NoError(err)
	A.True(env.IsWorkflowCompleted())
	A.NoError(env.GetWorkflowError())
}

Specifications

  • Version:
  • Platform:
@mnichols mnichols added the bug Something isn't working label Mar 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant