Skip to content

Conversation

@javier-godoy
Copy link
Member

@javier-godoy javier-godoy commented Jan 23, 2026

Close #107

What this PR does

This PR fixes intermittent failures in SelectionFeatureIT and other integration tests by ensuring the terminal's write queue is flushed after each sendKeys() call.

The Problem

xterm.js processes terminal.write() calls asynchronously for performance. When our key handlers call terminal.write() with escape sequences, the buffer update happens later in the event loop. Our test assertions were sometimes running before the buffer was updated, causing flaky failures.

The Fix

Modified XTermElement.sendKeys() to use executeAsyncScript with xterm's write callback:

@Override
public void sendKeys(CharSequence... keysToSend) {
    input.sendKeys(keysToSend);
    executeAsyncScript(
        "var callback = arguments[arguments.length - 1]; " +
        "this.terminal.write('', callback);"
    );
}

The empty write('') with a callback ensures all pending writes are processed before the script returns, making the test deterministic.

Why not Thread.sleep() or waitUntil()?

  • Thread.sleep(): Works but is non-deterministic (and slows down tests unnecessarily)
  • waitUntil(): Doesn't help because the buffer value doesn't change between polls until the write queue is flushed. The polling reads the same stale buffer value repeatedly and there's no observable state change to wait for.

Testing

  • Ran SelectionFeatureIT multiple times consecutively with no failures

Summary by CodeRabbit

  • Tests
    • Expanded selection feature test suite with additional test scenarios covering text selection, cursor positioning, and deletion operations.
    • Enhanced test synchronization to ensure proper Vaadin readiness before test execution.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 23, 2026

Walkthrough

The changes address flaky integration tests caused by asynchronous terminal write processing in the XTerm add-on. Test infrastructure is updated to include Vaadin readiness checks, test scenarios are expanded into separate numbered methods for better isolation, and the element's sendKeys method is enhanced with an async callback to ensure terminal write completion. Copyright years are updated to 2026 across affected files.

Changes

Cohort / File(s) Summary
Test Infrastructure Setup
src/test/java/com/flowingcode/vaadin/addons/xterm/integration/AbstractViewTest.java
Updated copyright to 2026; added getCommandExecutor().waitForVaadin() call in test setup to ensure Vaadin readiness before test execution proceeds.
Test Scenario Expansion
src/test/java/com/flowingcode/vaadin/addons/xterm/integration/SelectionFeatureIT.java
Renamed original testSelectionFeature() to testSelectionFeature1() and added four new test methods (testSelectionFeature2 through testSelectionFeature5) that isolate distinct selection, cursor position, and deletion scenarios with independent test initialization and assertion patterns.
Terminal Write Async Handling
src/test/java/com/flowingcode/vaadin/addons/xterm/integration/XTermElement.java
Updated copyright to 2020-2026; enhanced sendKeys() method with asynchronous script callback that invokes terminal write with callback to signal completion and ensure buffer updates are processed before test assertions.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'test: fix flaky SelectionFeatureIT test' accurately summarizes the main objective: fixing flakiness in the SelectionFeatureIT test, which is the core focus of this PR.
Linked Issues check ✅ Passed The PR successfully addresses all coding requirements from issue #107: adding waitForVaadin synchronization, flushing terminal write queue via executeAsyncScript callback in sendKeys, expanding test coverage via multiple test methods, and achieving deterministic behavior without non-deterministic sleeps.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing flaky integration tests: copyright year updates are maintenance, AbstractViewTest adds Vaadin synchronization, XTermElement implements write-queue flush logic, and SelectionFeatureIT expands test scenarios—all aligned with issue #107 objectives.

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

✨ Finishing touches
  • 📝 Generate docstrings

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.

@sonarqubecloud
Copy link

@javier-godoy javier-godoy marked this pull request as ready for review January 23, 2026 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: To Do

Development

Successfully merging this pull request may close these issues.

Flaky Integration Tests Due to Async Terminal Write Processing

2 participants