test: Fix flaky If-Else component reset regression test

[Cristhianzl]

This pull request updates the regression test for the flow run reset functionality, focusing on the If-Else component. The changes improve test reliability and better reflect the intended user flow when running the component multiple times with different branches.

Test scenario update:

• Changed the test description to clarify that it now verifies running a flow with the If-Else component multiple times, rather than sending images in the playground.

Test stability improvements:

• Added multiple waitForTimeout calls to ensure UI elements and state are ready before performing actions, reducing flakiness in the test.

Summary by CodeRabbit

• Tests
  • Enhanced regression testing for If-Else flow execution across multiple branch scenarios with improved test stability.

[coderabbitai]

Walkthrough

A regression test for If-Else flow execution is refactored. The test focus shifts from image sending in the playground to validating multiple runs with different branches. Deliberate 500ms waits are added throughout to improve test stability.

Changes

Cohort / File(s)	Summary
Regression Test Refactoring src/frontend/tests/extended/regression/general-bugs-reset-flow-run.spec.ts	Test renamed and refocused from image sending validation to If-Else flow multi-branch execution. Multiple 500ms wait() calls inserted around input actions and retesting sequences to enhance timing reliability.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

bug, lgtm

Suggested reviewers

• lucaseduoli

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.
Test Quality And Coverage	⚠️ Warning	The test successfully covers the main functionality for validating If-Else component behavior by running multiple scenarios with different inputs to test both branches, which is appropriate for a regression test. It properly follows Playwright patterns for frontend testing and includes explicit assertions that count successful and inactive component runs. However, the test quality is compromised by reliance on hard-coded waitForTimeout(500) calls throughout. According to official Playwright documentation and industry best practices, using waitForTimeout is inherently flaky and should never be used in production tests; instead, developers should use Locator actions and web assertions that wait automatically. The test contains at least 6 instances of waitForTimeout(500) which violate best practices because hard timeouts are an anti-pattern, as they lower performance, increase the chances of a script breaking, and often introduce test flakiness. The test also has limited behavioral validation—it only checks that component run counts equal 1, rather than validating the actual output values or state changes resulting from different branches.	Replace all waitForTimeout(500) calls with Playwright's auto-waiting mechanisms. For assertions, use auto-retrying assertions whenever possible such as expect(locator).toBeVisible() or expect(locator).toHaveText() which include built-in retry logic. For complex conditions like verifying component counts, use expect.poll() to wrap the count logic with automatic retries. Additionally, enhance the test's validation depth by asserting actual output values or component states when different branches execute, rather than only checking that components ran. This will transform the test from a smoke test into comprehensive behavior validation that properly reflects the intended user flow.

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The pull request title "test: Fix flaky If-Else component reset regression test" directly and clearly aligns with the main changes documented in the PR. The summary confirms that the PR updates a regression test for the If-Else component reset functionality and improves its stability by adding wait times to reduce flakiness. The title accurately captures this primary purpose, is concise and specific, follows standard commit message conventions with the "test:" prefix, and contains no vague or misleading language. A developer scanning the repository history would immediately understand that this PR addresses test reliability for the If-Else reset functionality.
Test Coverage For New Implementations	✅ Passed	The PR includes a regression test that properly addresses the bug fix for the If-Else component reset flow functionality. The test file general-bugs-reset-flow-run.spec.ts follows the correct naming convention (.spec.ts suffix) and contains substantial, non-placeholder test logic. The test creates a flow with an If-Else component connected to two Text Output components, then validates that the flow can be run multiple times with different inputs (triggering different branches each time), verifying that component states are properly reset between runs via explicit assertions on the count of active/inactive components. The improvements made (adding waitForTimeout calls) are stability enhancements to the existing regression test, not replacements of core functionality, making this a legitimate bug fix with appropriate test coverage.
Test File Naming And Structure	✅ Passed	The test file follows correct Playwright patterns and structure. The file is properly named general-bugs-reset-flow-run.spec.ts using the .spec.ts extension, which is the standard Playwright convention used throughout this repository. It is located in src/frontend/tests/extended/regression/, clearly marking it as a regression test in the frontend test suite. The test has a highly descriptive name: "user can run flow with If-Else component multiple times with different branches" that clearly explains what is being tested. The test uses proper Playwright syntax with expect() and test() from custom fixtures, proper page API methods, and async/await structure. The test is well-organized with logical sections marked by comments, uses helper functions for setup (awaitBootstrapTest(), addLegacyComponents(), adjustScreenView(), zoomOut()), and includes strategic waitForTimeout() calls for stability. The test covers multiple scenarios and branches: running the flow with input "1" (true branch), input "2" (false branch), and retesting both branches again to verify reset functionality. For each scenario, it validates both successful component execution counts and inactive component counts, effectively testing positive scenarios and implicit negative branch handling.
Excessive Mock Usage Warning	✅ Passed	The test file contains zero mock objects (no jest.mock, vitest, or sinon usage). It implements pure Playwright E2E testing by interacting with real UI elements using page.getByTestId(), page.click(), page.fill(), and page.dragTo(), then validating actual DOM element counts with real assertions. The fixtures.ts fixture extends Playwright's test only to add error monitoring for backend responses, which is appropriate testing infrastructure rather than mocking. The test validates authentic user workflows with the If-Else component running multiple times with different input branches, exercising real component logic instead of mocking behavior. This demonstrates proper test design that avoids the pitfalls of excessive mocking.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch cz/fix-resetflow

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[github-actions]

Frontend Unit Test Coverage Report

Coverage Summary

Lines	Statements	Branches	Functions
	10.85% (2894/26660)	4.49% (916/20360)	6.38% (373/5844)

Unit Test Results

Tests	Skipped	Failures	Errors	Time
1207	0 💤	0 ❌	0 🔥	14.232s ⏱️

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 29.99%. Comparing base (312ef9c) to head (7bad06f).
⚠️ Report is 6 commits behind head on main.

❌ Your project status has failed because the head coverage (39.42%) is below the target coverage (60.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #10380      +/-   ##
==========================================
- Coverage   29.99%   29.99%   -0.01%     
==========================================
  Files        1316     1316              
  Lines       59657    59661       +4     
  Branches     8921     8922       +1     
==========================================
+ Hits        17896    17897       +1     
- Misses      40944    40947       +3     
  Partials      817      817              

Flag	Coverage Δ
frontend	10.02% <ø> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

src/frontend/tests/extended/regression/general-bugs-reset-flow-run.spec.ts (2)

145-153: Consider replacing hardcoded timeouts with condition-based waits.

While these waitForTimeout calls may reduce flakiness, they represent an anti-pattern in E2E testing. The inconsistent timeout pattern (this section has 3 waits while others have 1-2) suggests the root cause of the flakiness may not be fully understood.

For more robust and maintainable tests, consider:

Recommended approach:

Replace hardcoded delays with condition-based waits that verify the UI is in the expected state:

-    await page.waitForTimeout(500);
-
     await page.getByTestId("popover-anchor-input-input_text").fill("1");
-    await page.waitForTimeout(500);
-
+    // Wait for input to be ready and value to be set
+    await expect(page.getByTestId("popover-anchor-input-input_text")).toHaveValue("1");
+
     await page.getByTestId("button_run_text output").click();
-
-    await page.waitForTimeout(500);

If timing is critical due to debouncing or animations, use page.waitForLoadState('networkidle') or wait for specific state changes rather than arbitrary timeouts.

Note: This suggestion applies to all waitForTimeout calls throughout the test (lines 78, 100, 121, 126, 145, 148, 152, 171).

---

106-189: Consider extracting repeated assertion logic into a helper function.

The assertion pattern is duplicated four times throughout the test, differing only in the component names. This reduces maintainability and readability.

Consider refactoring to a helper function:

async function assertBranchExecution(
  page: Page,
  activeComponent: string,
  inactiveComponent: string
) {
  const successfulCount = await page
    .getByTestId(`node_duration_${activeComponent}`)
    .count();
  const inactiveCount = await page
    .getByTestId(`node_status_icon_${inactiveComponent}_inactive`)
    .count();

  expect(successfulCount).toBe(1);
  expect(inactiveCount).toBe(1);
}

Then replace the duplicated blocks with:

// First run - true branch
await assertBranchExecution(page, "text output", "textoutputfalse");

// Second run - false branch  
await assertBranchExecution(page, "textoutputfalse", "text output");

// Third run - true branch retest
await assertBranchExecution(page, "text output", "textoutputfalse");

// Fourth run - false branch retest
await assertBranchExecution(page, "textoutputfalse", "text output");

This would make the test more maintainable and highlight the test's core logic.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e7f1a71 and 7bad06f.

📒 Files selected for processing (1)

• src/frontend/tests/extended/regression/general-bugs-reset-flow-run.spec.ts (2 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

src/frontend/**/*.@(test|spec).{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)

src/frontend/**/*.@(test|spec).{ts,tsx,js,jsx}: Frontend test files should be located in 'src/frontend/' and use '.test.{ts,tsx,js,jsx}' or '.spec.{ts,tsx,js,jsx}' extensions.
Test both sync and async code paths in frontend test files.
Mock external dependencies appropriately in frontend test files to isolate unit tests from external services.
Test error handling and edge cases in frontend test files.
Validate input/output behavior and test component initialization and configuration in frontend test files.
Each frontend test should have a clear description or comment explaining its purpose, especially for complex setups or mocks.

Files:

• src/frontend/tests/extended/regression/general-bugs-reset-flow-run.spec.ts

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (43)

• GitHub Check: Run Frontend Unit Tests / Frontend Jest Unit Tests
• GitHub Check: Run Frontend Tests / Determine Test Suites and Shard Distribution
• GitHub Check: Test Starter Templates
• GitHub Check: Playwright Tests - Shard 40/40
• GitHub Check: Playwright Tests - Shard 39/40
• GitHub Check: Playwright Tests - Shard 38/40
• GitHub Check: Playwright Tests - Shard 33/40
• GitHub Check: Playwright Tests - Shard 37/40
• GitHub Check: Playwright Tests - Shard 35/40
• GitHub Check: Playwright Tests - Shard 36/40
• GitHub Check: Playwright Tests - Shard 29/40
• GitHub Check: Playwright Tests - Shard 32/40
• GitHub Check: Playwright Tests - Shard 31/40
• GitHub Check: Playwright Tests - Shard 34/40
• GitHub Check: Playwright Tests - Shard 30/40
• GitHub Check: Playwright Tests - Shard 28/40
• GitHub Check: Playwright Tests - Shard 26/40
• GitHub Check: Playwright Tests - Shard 23/40
• GitHub Check: Playwright Tests - Shard 24/40
• GitHub Check: Playwright Tests - Shard 21/40
• GitHub Check: Playwright Tests - Shard 25/40
• GitHub Check: Playwright Tests - Shard 27/40
• GitHub Check: Playwright Tests - Shard 11/40
• GitHub Check: Playwright Tests - Shard 22/40
• GitHub Check: Playwright Tests - Shard 19/40
• GitHub Check: Playwright Tests - Shard 17/40
• GitHub Check: Playwright Tests - Shard 16/40
• GitHub Check: Playwright Tests - Shard 12/40
• GitHub Check: Playwright Tests - Shard 7/40
• GitHub Check: Playwright Tests - Shard 18/40
• GitHub Check: Playwright Tests - Shard 15/40
• GitHub Check: Playwright Tests - Shard 20/40
• GitHub Check: Playwright Tests - Shard 9/40
• GitHub Check: Playwright Tests - Shard 6/40
• GitHub Check: Playwright Tests - Shard 5/40
• GitHub Check: Playwright Tests - Shard 10/40
• GitHub Check: Playwright Tests - Shard 13/40
• GitHub Check: Playwright Tests - Shard 8/40
• GitHub Check: Playwright Tests - Shard 3/40
• GitHub Check: Playwright Tests - Shard 14/40
• GitHub Check: Playwright Tests - Shard 2/40
• GitHub Check: Playwright Tests - Shard 1/40
• GitHub Check: Playwright Tests - Shard 4/40

🔇 Additional comments (1)

src/frontend/tests/extended/regression/general-bugs-reset-flow-run.spec.ts (1)

11-11: Improved test description clarity.

The updated test name accurately describes the test's purpose: validating that the If-Else component can be executed multiple times across different branches. This is clearer than the previous description.