Skip to content

test(cli): raise i18n ToolMessage test timeout to 15s to stop merge-queue flake #5858

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

Merged
yiliang114 merged 1 commit into from
Jun 25, 2026
Merged

test(cli): raise i18n ToolMessage test timeout to 15s to stop merge-queue flake #5858

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions packages/cli/src/ui/components/messages/ToolMessage.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -995,7 +995,10 @@ describe('<ToolMessage /> localized badge', () => {
const output = lastFrame() ?? '';
expect(output).toContain('读取文件');
expect(output).not.toContain('ReadFile');
});
// 15s timeout (not the 5s default): setLanguageAsync() loads locale

@wenshao wenshao Jun 25, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] The comment explaining the 15s timeout sits inside the zh test body (between assertions and }, 15000)), while the en test at line 1011 has }, 15000) with no explanation. A future maintainer grepping for 15000 will see one test with rationale and one without — inviting either removal of the "unnecessary" timeout or wasted investigation.

The en test does need 15s when run in isolation (e.g., vitest -t "keeps the English") because no prior afterEach has warmed translationCache['en']. But this rationale exists only in the PR description, not in the source.

Consider moving the comment above the it( call and adding one line covering both tests:

// 15s timeout (not the 5s default): setLanguageAsync() loads locale
// resources lazily and intermittently exceeds 5s on heavily parallelized
// macOS CI. The en test also cold-loads when run in isolation via -t.
it('shows the localized display name under the zh locale', async () => {
  // ...
}, 15000);

— qwen3.7-max via Qwen Code /review

// resources lazily and intermittently exceeds 5s on the heavily
// parallelized macOS CI runner, flaking the merge queue.
}, 15000);

it('keeps the English display name under the en locale', async () => {
const { setLanguageAsync } = await import('../../../i18n/index.js');
Expand All @@ -1005,5 +1008,5 @@ describe('<ToolMessage /> localized badge', () => {
StreamingState.Idle,
);
expect(lastFrame() ?? '').toContain('ReadFile');
});
}, 15000);

@qwen-code-ci-bot qwen-code-ci-bot Jun 25, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] The English-locale test may not need the 15s timeout bump. By the time this test runs, afterEach has already called setLanguageAsync('en') and populated translationCache['en'] — so setLanguageAsync('en') here is a cache hit and the dynamic import() (the slow path) never fires.

If the en test ever becomes slow for a non-i18n reason (rendering, test infra), the 15s ceiling would mask it. Consider reverting this test to the default 5s timeout and only keeping the bump on the zh test.

Suggested change
}, 15000);
});

— qwen3.7-max via Qwen Code /review

});
Loading