Skip to content

fix(test): update LoadingIndicator snapshot for correct output alignment#2469

Merged
tanzhenxin merged 1 commit intoQwenLM:mainfrom
qqqys:fix/fix_test_case
Mar 19, 2026
Merged

fix(test): update LoadingIndicator snapshot for correct output alignment#2469
tanzhenxin merged 1 commit intoQwenLM:mainfrom
qqqys:fix/fix_test_case

Conversation

@qqqys
Copy link
Copy Markdown
Collaborator

@qqqys qqqys commented Mar 19, 2026

TLDR

This PR fixes the LoadingIndicator test snapshot to match the current component output format. The snapshot was outdated and needed to be updated to reflect the correct text alignment/indentation in the component rendered output.

Dive Deeper

The LoadingIndicator component snapshot test was failing because the expected output format had changed. The snapshot now correctly captures:

  • Proper indentation with leading spaces
  • Correct alignment of the spinner and text elements
  • Accurate representation of the truncated text layout on 80-column terminals

This is a test-only change with no modifications to the actual component logic.

Reviewer Test Plan

  1. Run the test suite: npm run test -- LoadingIndicator.test.tsx
  2. Verify the test passes with the updated snapshot
  3. Optionally, review the snapshot diff to confirm the alignment fix is correct

Testing Matrix

🍏 🪟 🐧
npm run
npx
Docker
Podman - -
Seatbelt - -

Linked issues / bugs

No linked issues

@github-actions
Copy link
Copy Markdown
Contributor

📋 Review Summary

This PR updates the LoadingIndicator snapshot test to reflect the correct text alignment with proper indentation. The change adds 2-space left padding to match the component's actual rendered output, which uses <Box paddingLeft={2}> in the LoadingIndicator component. This is a straightforward test-only fix with no production code changes.

🔍 General Feedback

  • The snapshot update correctly addresses the misalignment between the expected and actual component output
  • The test change is minimal and focused, which is appropriate for snapshot fixes
  • The PR description clearly explains the rationale and provides a good testing plan
  • The change aligns the snapshot with the component's paddingLeft={2} styling defined in LoadingIndicator.tsx:53

🎯 Specific Feedback

🔵 Low

  • File: packages/cli/src/ui/components/__snapshots__/LoadingIndicator.test.tsx.snap:4-5 - The snapshot shows the text is being truncated at "truncated in" instead of "truncated in t" (the old snapshot had an extra "t" at the end). This appears to be a minor truncation boundary difference. Consider verifying the truncation behavior is consistent with the wrap="truncate-end" prop on the Text component to ensure the truncation point is intentional and not a side effect of terminal width calculation differences.

✅ Highlights

  • Clear and concise PR description with proper context about what changed and why
  • Appropriate test-only change for a snapshot mismatch issue
  • Good testing matrix provided in the PR description
  • The snapshot now correctly reflects the component's padding structure with the 2-space indentation matching <Box paddingLeft={2}>

@tanzhenxin tanzhenxin merged commit 59f167b into QwenLM:main Mar 19, 2026
15 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.

2 participants