Skip to content

test: stabilize activity list pagination - #1171

Merged
ks-temporal merged 2 commits into
temporalio:mainfrom
chaptersix:codex/activity-list-pagination-stable
Sep 1, 2026
Merged

test: stabilize activity list pagination#1171
ks-temporal merged 2 commits into
temporalio:mainfrom
chaptersix:codex/activity-list-pagination-stable

Conversation

@chaptersix

Copy link
Copy Markdown
Contributor

Related issues

N/A

What changed?

Make TestActivity_List_Pagination exercise a deterministic mix of three completed and two running standalone Activities.

The test now waits for completed results, blocks running Activities until pagination finishes, and waits for visibility to report the exact status mix before listing. It also verifies each Activity appears exactly once with the expected status, avoiding the race where an Activity could complete and move in visibility ordering between page requests.

Checklist

Stability

  • No user-facing behavior or output changes

Tests

  • Updated functional test (SharedServerSuite)
  • Focused pagination test passed 20 consecutive runs
  • Neighboring Activity list/count tests passed

Testing

go test ./internal/temporalcli -run 'TestSharedServerSuite/TestActivity_List_Pagination$' -count=20
go test ./internal/temporalcli -run 'TestSharedServerSuite/TestActivity_(List(_Pagination)?|Count)$' -count=1
git diff --check

@chaptersix
chaptersix marked this pull request as ready for review August 20, 2026 15:25
@chaptersix
chaptersix requested a review from a team as a code owner August 20, 2026 15:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The updated test asserts text output contains “Completed”/“Running”, but the CLI table output uses enum strings (e.g., ACTIVITY_EXECUTION_STATUS_COMPLETED), which will cause the test to fail.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR updates the TestActivity_List_Pagination functional test to make pagination assertions deterministic by controlling the mix of completed vs. running standalone Activities and by waiting for visibility to reflect that stable mix before listing.

Changes:

  • Creates a deterministic set of 3 completed and 2 running Activities and blocks the running ones until pagination checks finish.
  • Waits for visibility JSON output to show the expected status mix before executing pagination assertions.
  • Strengthens assertions to verify each expected Activity appears exactly once.
File summaries
File Description
internal/temporalcli/commands.activity_test.go Refactors the pagination test to reduce race/flakiness by coordinating activity execution and verifying stable visibility ordering/statuses.
Review details

Suppressed comments (1)

internal/temporalcli/commands.activity_test.go:1910

  • Same issue as above for running activities: the table output uses the enum name (ACTIVITY_EXECUTION_STATUS_RUNNING), not the title-cased shorthand "Running".
	for _, activityID := range runningActivityIDs {
		s.ContainsOnSameLine(out, "Running", activityID)
		s.Equal(1, strings.Count(out, activityID))
	}
  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread internal/temporalcli/commands.activity_test.go
@ks-temporal
ks-temporal added this pull request to the merge queue Sep 1, 2026
Merged via the queue into temporalio:main with commit afcf89c Sep 1, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants