fix: restore data-testid="chat-interface" removed by #340#437
Merged
Conversation
PR #340 added left padding to the ChatInterface wrapper div but accidentally dropped the data-testid attribute in the same edit. This broke both the collapsible-thinking snapshot tests and the live e2e test, which both use getByTestId('chat-interface') as the load signal and screenshot target. Co-authored-by: openhands <openhands@all-hands.dev>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
all-hands-bot
approved these changes
May 14, 2026
Contributor
all-hands-bot
left a comment
all-hands-bot
left a comment
There was a problem hiding this comment.
LGTM! 🟢
Clean fix that restores the accidentally removed test-id. The collapsible-thinking snapshot updates are appropriate.
Note: The settings-page snapshots were also updated here (analytics consent modal, home screen, settings page, settings app page). Your description mentions these should be done separately, but including them is fine if they were generated in CI. If you generated these locally, CI might reject them due to rendering differences (fonts, OS). If CI fails, just regenerate via the workflow with update_snapshots=true as you mentioned.
Risk Assessment: 🟢 LOW - Test-only change, no production logic impact.
… ruleset
The branch ruleset requires a status check named 'test-and-build', but the
CI job's 'name: Test and build (${{ matrix.os }})' field caused GitHub to
report the check as 'Test and build (ubuntu-24.04)' — so the required check
was never satisfied and PRs could not merge.
The matrix only ever had a single entry (ubuntu-24.04), so drop it and the
custom name field entirely. Without a 'name:' override, GitHub falls back to
the job key ('test-and-build') as the check run name, matching the ruleset.
Co-authored-by: openhands <openhands@all-hands.dev>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Restores the
data-testid="chat-interface"attribute that was accidentally dropped in #340.Root cause
PR #340 added left padding (
pl-0 md:pl-4) to theChatInterfacewrapper div and rewrote the element in a single line, silently removing thedata-testidattribute in the same edit:Impact
Two test paths depend on
getByTestId('chat-interface'):tests/e2e/snapshots/collapsible-thinking.snapshot.spec.ts– uses it as both the "conversation page loaded" signal and the screenshot target. All 4 collapsible-thinking snapshot tests failed (the first timed out after 20 s, the remaining 3 were skipped by serial mode).tests/e2e/live/real-agent-server-conversation.spec.ts– useswaitForTestId(page, "chat-interface")andpage.getByTestId("chat-interface").screenshot(...).Fix
Restore the attribute while keeping the new left-padding class from #340. No other changes.
Note on remaining snapshot failures: The 4
settings-page.snapshot.spec.tsfailures (analytics consent modal, home page, settings page, settings app page) are stale baselines — the UI changed visually across #340, #386, and #427. After this PR merges, trigger a baseline update via the Snapshot Tests workflow withupdate_snapshots=true.This PR was created by an AI agent (OpenHands) on behalf of the user.
@malhotra5 can click here to continue refining the PR