Worker-Versioning GA: Tests for AutoUpgrade workflows not bouncing back and forth. #8635
Conversation
tests/versioning_3_test.go
Outdated
| }}, []string{}, tqTypeWf, tqTypeAct) | ||
|
|
||
| // Wait until all task queue partitions know that v1 is current. | ||
| s.waitForDeploymentDataPropagation(tv1, versionStatusCurrent, false, tqTypeWf, tqTypeAct) |
There was a problem hiding this comment.
btw, later when we clean up all old tests we should maybe change this function to wait for the particular revision number propagation rather than status of the version.
There was a problem hiding this comment.
IMHO I think this test should be thinking of "propagation complete" == "the right revision number has been synced to matching partitions. We should have tests in the worker_deployment_suite that should test out if the right revision number is synced to these partitions alongside the statuses of each version.
I think having this differences, given we have two suites, would make test writing simpler and easier to read for a new user.
tests/versioning_3_test.go
Outdated
|
|
||
| func (s *Versioning3Suite) TestAutoUpgradeWorkflows_NoBouncingBetweenVersions() { | ||
| s.T().Skip("This test is flaky right now and shall be fixed in a future PR.") // TODO (Shivam) | ||
| if !s.useNewDeploymentData { |
…llWorkflow function so that it can terminate early
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| } | ||
|
|
||
| func (s *Versioning3Suite) idlePollWorkflow( | ||
| ctx context.Context, |
There was a problem hiding this comment.
this function shall now take in a context. The reason this was done is as follows:
There are some tests that kickstart a v0 poller (v0 is just an example) for the sole purpose of checking if a task were to ever go this worker. These tests initialize them in a waitGroup and then towards the end of the test, just before it gets cleaned up, there is a wg.Wait() present which ensures that this poller completes running before the test can be terminated safely.
However, this does mean that the minimum wait time for the test will never be lesser than the minimum poll time of this v0 poller. This makes our tests long to run. Thus, passing this context and cancelling it is a safe and efficient way to terminate the test.
| } | ||
|
|
||
| func (s *Versioning3Suite) idlePollWorkflow( | ||
| ctx context.Context, |
What changed?
Why?
How did you test it?
Potential risks
Note
Enables and rewrites AutoUpgrade no-bounce tests using revision numbers, adds several new lag/child/CAN/retry tests, and updates idle workflow poller to accept context for cancellable polling.
TestAutoUpgradeWorkflows_NoBouncingBetweenVersionsusing revision-number mechanics and cancellable idle pollers.TestWorkflowTQLags_DependentActivityStartsTransition,TestActivityTQLags_DependentActivityCompletesOnTheNewVersion.TestChildStartsWithParentRevision_SameTQ_TQAhead,TestChildStartsWithParentRevision_SameTQ_TQLags,TestChildStartsWithNoInheritedAutoUpgradeInfo_CrossTQ.TestContinueAsNewOfAutoUpgradeWorkflow_RevisionNumberMechanics,testRetryNoBounceBack(and callers) to avoid bounce-back under rollback.useRevisionNumbersinstead ofuseNewDeploymentData.idlePollWorkflownow takescontext.Contextand passes it viataskpoller.WithContext, enabling time-bounded idle pollers; update all call sites accordingly.sync.WaitGroupuses in favor of context-based cancellation/timeouts.Written by Cursor Bugbot for commit 3a6fab3. This will update automatically on new commits. Configure here.