Skip to content

[Scheduled Actions] Clean up test suites' addedTasks checks #8283

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[Scheduled Actions] Clean up test suites' addedTasks checks #8283

wants to merge 1 commit into from
+22 −18

Conversation

lina-temporal
Copy link
Contributor

What changed?

  • Cleaned up s.addedTasks within test suites

Why?

  • In preparation for adding the Visibility component, which itself also adds tasks (breaking some of the existing asserts).
  • I manually verified that the backfiller is properly generating rescheduled tasks and that they validate in CloseTransaction.

How did you test it?

  • built
  • run locally and tested manually
  • covered by existing tests
  • added new unit test(s)
  • added new functional test(s)

@lina-temporal lina-temporal requested a review from yycptt September 4, 2025 15:45
@lina-temporal lina-temporal requested a review from a team as a code owner September 4, 2025 15:45
yycptt
yycptt approved these changes Sep 9, 2025
View reviewed changes
chasm/lib/scheduler/backfiller_tasks_test.go
task, ok := s.addedTasks[0].(*tasks.ChasmTaskPure)
s.True(ok)
s.Equal(chasm.TaskScheduledTimeImmediate, task.GetVisibilityTime())
s.True(s.hasTask(&tasks.ChasmTaskPure{}, chasm.TaskScheduledTimeImmediate))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not related to this PR, but I don't think transfer queue supports pure task. That combination doesn't really make sense I think and the pure task logic can be executed in the same transaction instead.

yycptt
yycptt reviewed Sep 9, 2025
View reviewed changes
Copy link
Member

@yycptt yycptt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved this PR but we need to sync offline re. immediate pure task.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yycptt yycptt yycptt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lina-temporal @yycptt