Skip to content

Fix flakey SchemaDialogView 'change text' JS test#9723

Open
dpage wants to merge 1 commit intopgadmin-org:masterfrom
dpage:fix_flakey_test
Open

Fix flakey SchemaDialogView 'change text' JS test#9723
dpage wants to merge 1 commit intopgadmin-org:masterfrom
dpage:fix_flakey_test

Conversation

@dpage
Copy link
Contributor

@dpage dpage commented Mar 10, 2026

Add a wait for the FormView autofocus timer (200ms) to complete before typing, preventing a race condition where the autofocus moves focus away from the target field on slow CI machines. This matches the pattern already used by simulateValidData in the same test file.

Summary by CodeRabbit

  • Tests
    • Improved timing reliability in schema dialog view test by adjusting focus-related test execution flow.

Add a wait for the FormView autofocus timer (200ms) to complete before
typing, preventing a race condition where the autofocus moves focus away
from the target field on slow CI machines. This matches the pattern
already used by simulateValidData in the same test file.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link

coderabbitai bot commented Mar 10, 2026

Walkthrough

The test file SchemaDialogView.spec.js is modified to add a 500ms asynchronous delay within an act wrapper before a type action. This change accommodates UI autofocus timing in the "change text" test case, ensuring subsequent keyboard input occurs after focus state completion.

Changes

Cohort / File(s) Summary
Test Timing Fix
web/regression/javascript/SchemaView/SchemaDialogView.spec.js
Added 500ms delay within act wrapper before typing into field2 to wait for autofocus timer completion.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: fixing a flaky test in SchemaDialogView by adding a wait for autofocus timing, which directly matches the changeset's purpose.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
web/regression/javascript/SchemaView/SchemaDialogView.spec.js (1)

104-107: Extract the autofocus settle delay into a helper.

This fixes the race, but it adds another copy of the same 500ms sleep already used in simulateValidData. A shared helper/constant would keep the FormView timing assumption in one place and make future timer changes less error-prone.

♻️ Suggested cleanup
+    const waitForAutofocus = async ()=>{
+      await act(async ()=>{
+        await new Promise(resolve => setTimeout(resolve, 500));
+      });
+    };
+
     describe('form fields', ()=>{
@@
       it('change text', async ()=>{
-        // Wait for autofocus timer (200ms in FormView) to complete
-        await act(async ()=>{
-          await new Promise(resolve => setTimeout(resolve, 500));
-        });
+        await waitForAutofocus();
         await user.type(ctrl.container.querySelector('[name="field2"]'), '2');
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/regression/javascript/SchemaView/SchemaDialogView.spec.js` around lines
104 - 107, The test duplicates a 500ms sleep to wait for FormView autofocus;
extract that delay into a shared helper or constant so both this test and
simulateValidData reference the same value. Create a named constant (e.g.,
AUTOFOCUS_SETTLE_MS) or a helper function (e.g., waitForAutofocus or waitMs) and
replace the inline new Promise(setTimeout) usage in the act block and inside
simulateValidData to use that helper/constant, keeping the FormView timing
assumption in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@web/regression/javascript/SchemaView/SchemaDialogView.spec.js`:
- Around line 104-107: The test duplicates a 500ms sleep to wait for FormView
autofocus; extract that delay into a shared helper or constant so both this test
and simulateValidData reference the same value. Create a named constant (e.g.,
AUTOFOCUS_SETTLE_MS) or a helper function (e.g., waitForAutofocus or waitMs) and
replace the inline new Promise(setTimeout) usage in the act block and inside
simulateValidData to use that helper/constant, keeping the FormView timing
assumption in one place.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 8dd73d3e-13b2-4c00-b9f2-2e2e56614883

📥 Commits

Reviewing files that changed from the base of the PR and between a0e6da0 and 265112c.

📒 Files selected for processing (1)
  • web/regression/javascript/SchemaView/SchemaDialogView.spec.js

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