Migrate sizelimit tests to use TestEnv#9720
Migrate sizelimit tests to use TestEnv#9720chaptersix wants to merge 6 commits intotemporalio:mainfrom
Conversation
|
Use hide white space filter in diff view. |
chaptersix
force-pushed
the
sizelimit-test-env
branch
from
6415fab to
fa3c358
Compare
tests/sizelimit_test.go
Outdated
| s.FunctionalTestBase.SetupSuiteWithCluster(testcore.WithDynamicConfigOverrides(dynamicConfigOverrides)) | ||
| } | ||
| // TODO use parallel suite | ||
| t.Run("TerminateWorkflowCausedByHistoryCountLimit", func(t *testing.T) { |
There was a problem hiding this comment.
Is there an advantage of inlining all the tests vs. keeping them top-level functions, and doing something like this?
func TestSizeLimitFunctionalSuite(t *testing.T) {
...
t.Run(
"TerminateWorkflowCausedByHistoryCountLimit",
TestTerminateWorkflowCausedByHistoryCountLimit,
)
...
That way the old test implementations don't need to be indented and can be mostly intact from git's POV.
Asking because a lot of these indentation-only changes due to inlining can make git blame a bit more confusing to dig through
There was a problem hiding this comment.
I each sub test is shared with the whole package. We want to use suites but testify had some drawbacks. I'm about to move this over to parallel suite. (just merged)
There was a problem hiding this comment.
git blame is already not all that reliable in terms of figuring out who to ask about the test. people move around quite a bit.
- Add space in // TODO comment (revive) - Migrate from deprecated testcore.TaskPoller to taskpoller.TaskPoller - Replace s.IsType with s.ErrorAs (testifylint) - Add context cancellation to single-case select statements - Remove unreachable activitiesScheduled branch in MsSizeLimit test
Convert from t.Run subtests to parallelsuite.Suite methods for parallel test execution. Use direct gRPC calls for activity polling in HistoryCountLimit test due to taskpoller.PollAndHandleActivityTask bug.
chaptersix
force-pushed
the
sizelimit-test-env
branch
from
fa3c358 to
5ff456f
Compare
tests/sizelimit_test.go
Outdated
| // NOTE: Using direct gRPC calls for activity polling because taskpoller.PollAndHandleActivityTask | ||
| // has a bug where it doesn't find activity tasks that are immediately available after workflow task completion. | ||
| actResp, actErr := env.FrontendClient().PollActivityTaskQueue(testcore.NewContext(), &workflowservice.PollActivityTaskQueueRequest{ | ||
| Namespace: env.Namespace().String(), | ||
| TaskQueue: taskQueue, | ||
| Identity: identity, | ||
| }) | ||
| s.NoError(actErr) | ||
| s.NotEmpty(actResp.GetTaskToken()) | ||
| _, actErr = env.FrontendClient().RespondActivityTaskCompleted(testcore.NewContext(), &workflowservice.RespondActivityTaskCompletedRequest{ | ||
| Namespace: env.Namespace().String(), | ||
| TaskToken: actResp.TaskToken, | ||
| Identity: identity, | ||
| Result: payloads.EncodeString("Activity Result"), | ||
| }) | ||
| s.NoError(actErr) |
There was a problem hiding this comment.
claude is struggle busing with this one. I don't like the idea of swapping over to GRPC. seems like it's hiding something.
Keep each sizelimit test focused on its own limit and document that longer poller identities can trip mutable-state/history-size limits before history-count limits.
| WorkflowRunTimeout: durationpb.New(100 * time.Second), | ||
| WorkflowTaskTimeout: durationpb.New(1 * time.Second), | ||
| Identity: identity, | ||
| Identity: testcore.RandomizeStr("worker"), |
There was a problem hiding this comment.
nit: I think this can just be hard-coded to whatever you like since the test runs in an isolation ns and isolated worker
3 participants
What changed?
tests/sizelimit_test.gofromtestify/suite+FunctionalTestBasesetup totestcore.TestEnv*testcore.TestEnvt.Runand removed suite wiring/importsizeLimitTestOptshelper with the same dynamic config overrides used by the old suite setupWhy?
Part of the ongoing piece-by-piece migration of functional tests to
TestEnv, while keeping this change minimal for easier review.How did you test it?
make lintgo test -tags disable_grpc_modules,test_dep -timeout 5m -run TestSizeLimitFunctionalSuite ./tests/...Potential risks
Tests only.