Skip to content

test(setup): update DictationDemo asset-missing contract to #294 (fixes CI on main) - #302

Merged
debpalash merged 1 commit into
mainfrom
fix/dictation-demo-test
Jun 7, 2026
Merged

test(setup): update DictationDemo asset-missing contract to #294 (fixes CI on main)#302
debpalash merged 1 commit into
mainfrom
fix/dictation-demo-test

Conversation

@debpalash

@debpalash debpalash commented Jun 7, 2026

Copy link
Copy Markdown
Owner

CI on main has been red since #294: DictationDemo.test.jsx asserted the component renders empty when demo clips 404 — the exact behavior #294 deliberately changed (blank Try-dictation act → always-on hotkey card). Test now asserts the new contract: script cards asset-gated, hotkey card stays. Full suite: 34 files / 196 tests green.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced handling of missing sample assets in the demo. The demo interface now remains visible with asset-dependent elements selectively hidden, providing a better user experience when sample assets are unavailable.

The test asserted the component renders nothing when demo clips 404 —
the exact behavior #294 deliberately removed (it blanked the wizard's
Try-dictation act on every real install). New contract under test: the
script cards are asset-gated and disappear; the hotkey card (shortcut +
press-to-verify, zero assets needed) stays.

This was the single failure breaking CI on main since #294 merged
(34 files / 196 tests green with the fix).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 1693ae81-aadb-413a-99f3-bb95680fe331

📥 Commits

Reviewing files that changed from the base of the PR and between 1171185 and 83cccd7.

📒 Files selected for processing (1)
  • frontend/src/test/DictationDemo.test.jsx

📝 Walkthrough

Walkthrough

Test update for DictationDemo assertion behavior when sample assets are unavailable. The "missing sample assets (HEAD 404)" scenario now verifies selective hiding of script cards container while preserving the demo root, instead of expecting full DOM removal.

Changes

DictationDemo Test Update

Layer / File(s) Summary
HEAD 404 test assertion update
frontend/src/test/DictationDemo.test.jsx
Test case for the HEAD 404 scenario changed assertions to verify .dictation-demo__scripts is hidden while .dictation-demo remains present, matching the updated UI behavior for asset-gated content.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Possibly related PRs

  • debpalash/OmniVoice-Studio#153: Introduced the selective asset-gating behavior that hides script cards on missing sample assets; this PR updates test assertions to match that behavior.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The description explains the context and rationale, but lacks completion of the required template sections (Type, Testing, Checklist, Screenshots). Complete the PR template by selecting the appropriate Type checkbox (likely 'Tests'), filling Testing section, and marking applicable Checklist items.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: updating a test contract related to issue #294 and fixing CI on main.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/dictation-demo-test

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps

greptile-apps Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a broken CI test by updating DictationDemo.test.jsx to match the component contract introduced in #294. The old test asserted the component rendered nothing when demo audio assets returned a HEAD 404; the new behavior (from #294) is that only the replayable script cards (dictation-demo__scripts) are hidden while the hotkey-teaching panel (dictation-demo) remains.

  • The updated assertion correctly reflects the component's logic: showScripts = assetsAvailable !== false, meaning the panel always renders but the scripts div is conditionally included only when assets are confirmed present.
  • All three assertions (.dictation-demo__scripts absent, .dictation-demo present, script text not in DOM) are accurate matches for what the component produces in the 404 case.

Confidence Score: 5/5

Safe to merge — the test-only change correctly tracks the component's current DOM shape with no risk to production code.

Single test file update: the three new assertions (.dictation-demo__scripts absent, .dictation-demo present, script text gone) are exact matches for what the component renders when assetsAvailable is false. The waitFor correctly polls for the async state transition, and the follow-up DOM check on the always-rendered section is safe outside the retry loop. No logic, dependency, or API surface has changed.

No files require special attention.

Important Files Changed

Filename Overview
frontend/src/test/DictationDemo.test.jsx Updated the asset-missing test case to assert the hotkey card stays visible while only .dictation-demo__scripts is hidden on HEAD 404 — accurately mirrors the #294 component change (showScripts = assetsAvailable !== false gate).

Sequence Diagram

sequenceDiagram
    participant Test as DictationDemo.test.jsx
    participant Comp as DictationDemo (mount)
    participant Fetch as global.fetch (mocked)

    Test->>Comp: "render(withI18n(<DictationDemo />))"
    Note over Comp: assetsAvailable = null → showScripts = true<br/>Both .dictation-demo and .dictation-demo__scripts rendered
    Comp->>Fetch: HEAD /demo_audio/dictation/en_conversational.wav
    Fetch-->>Comp: "{ ok: false, status: 404 }"
    Note over Comp: setAssetsAvailable(false) → showScripts = false<br/>.dictation-demo__scripts removed from DOM
    Test->>Comp: waitFor → .dictation-demo__scripts is null ✓
    Test->>Comp: .dictation-demo is not null ✓
    Test->>Comp: queryByText(/Schedule a meeting with Pat/) absent ✓
Loading

Reviews (1): Last reviewed commit: "test(setup): update DictationDemo asset-..." | Re-trigger Greptile

@debpalash
debpalash merged commit e424f46 into main Jun 7, 2026
15 checks passed
@debpalash
debpalash deleted the fix/dictation-demo-test branch June 12, 2026 10:10
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.

1 participant