Skip to content

Fix flaky Emacs e2e test: replace fixed sleep with diagnostic polling loop#2276

Merged
andyleejordan merged 2 commits intoupdatesfrom
copilot/fix-powershell-editor-services-test
Apr 8, 2026
Merged

Fix flaky Emacs e2e test: replace fixed sleep with diagnostic polling loop#2276
andyleejordan merged 2 commits intoupdatesfrom
copilot/fix-powershell-editor-services-test

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 8, 2026

The powershell-editor-services Emacs test was non-deterministically failing because it relied on a hardcoded (sleep-for 5) before checking for flymake diagnostics — insufficient when the LSP server publishes diagnostics slowly. The TODO comment in the code acknowledged this.

Changes

  • test/emacs-test.el: Replace (sleep-for 5) with a polling loop that waits up to 30 seconds (in 0.5s increments) for flymake-diagnostics to become non-nil before proceeding with flymake-goto-next-error and the face assertion.
(flymake-start)
;; Wait for diagnostics to arrive instead of sleeping a fixed duration
(let ((deadline (time-add (current-time) 30)))
  (while (and (time-less-p (current-time) deadline)
              (null (flymake-diagnostics)))
    (sleep-for 0.5)))
(goto-char (point-min))
(flymake-goto-next-error)
(should (eq 'flymake-warning (face-at-point)))
Original prompt

Problem

The Emacs End-to-End Tests workflow is failing on PR #2275. The test powershell-editor-services in test/emacs-test.el fails with:

(eq flymake-warning nil)
:value nil

The assertion on line 68 fails because face-at-point returns nil instead of flymake-warning. This is a timing issue — the test uses a hardcoded (sleep-for 5) before checking for flymake diagnostics, but the LSP server hasn't published diagnostics yet by the time the face is checked. There's even a TODO comment in the code acknowledging this:

(sleep-for 5) ; TODO: Wait for "textDocument/publishDiagnostics" instead

Fix

In test/emacs-test.el, replace the hardcoded (sleep-for 5) with a polling loop that waits until flymake-diagnostics is non-nil (up to a reasonable timeout, e.g. 30 seconds), then proceeds with flymake-goto-next-error and the assertion.

The fix should look something like this:

(flymake-start)
;; Wait for diagnostics to arrive instead of sleeping a fixed duration
(let ((deadline (time-add (current-time) 30)))
  (while (and (time-less-p (current-time) deadline)
              (null (flymake-diagnostics)))
    (sleep-for 0.5)))
(goto-char (point-min))
(flymake-goto-next-error)
(should (eq 'flymake-warning (face-at-point)))

This addresses the TODO comment and makes the test robust against variable server startup and diagnostic publish times.

The change should be pushed to the updates branch (the head branch of PR #2275) so the fix is included in that PR.

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: How can I fix this build?

Assistant: The build is failing in the Emacs End-to-End Tests workflow. The test powershell-editor-services is failing with:

(eq flymake-warning nil)
:value nil

This means face-at-point is returning nil instead of flymake-warning at line 68 of test/emacs-test.el:

(should (eq 'flymake-warning (face-at-point)))

The root cause is a timing issue. The test calls sleep-for 5 (line 64) before calling flymake-start, expecting diagnostics to be ready, but the flymake face hasn't been applied at point by the time face-at-point is checked. There's even a TODO comment acknowledging this:

(sleep-for 5) ; TODO: Wait for "textDocument/publishDiagnostics" instead

How to fix it

Replace the hardcoded sleep-for 5 with a proper poll/wait loop that checks for flymake diagnostics to actually be present before asserting. For example:

(flymake-start)
;; Wait for diagnostics to arrive instead of sleeping a fixed duration
(let ((deadline (time-add (current-time) 30)))
  (while (and (time-less-p (current-time) deadline)
              (null (flymake-diagnostics)))
    (sleep-for 0.5)))
(goto-char (point-min))
(flymake-goto-next-error)
(should (eq 'flymake-warning (face-at-point)))

This polls for up to 30 seconds in 0.5-second intervals until flymake-diagnostics is non-nil, making the test robust against variable server startup/diagnostic publish times rather than relying on an arbitrary fixed sleep.

User: Okay open a PR fixing it please

Assistant: We're waiting for your response on the tool call dialog to continue. Once you approve, the coding agent will make the fix directly to the updates branch of PR #2275.

This pull request was created from Copilot chat.

Copilot AI changed the title [WIP] Fix timing issue in powershell-editor-services test Fix flaky Emacs e2e test: replace fixed sleep with diagnostic polling loop Apr 8, 2026
Copilot AI requested a review from andyleejordan April 8, 2026 20:25
@andyleejordan andyleejordan marked this pull request as ready for review April 8, 2026 20:26
@andyleejordan andyleejordan merged commit 9455abe into updates Apr 8, 2026
1 check passed
@andyleejordan andyleejordan deleted the copilot/fix-powershell-editor-services-test branch April 8, 2026 20:27
andyleejordan added a commit that referenced this pull request Apr 8, 2026
* One more round of package updates

* Update test/PowerShellEditorServices.Test/PowerShellEditorServices.Test.csproj

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Fix flaky Emacs e2e test: replace fixed sleep with diagnostic polling loop (#2276)

* Initial plan

* Fix timing issue in Emacs test by polling for flymake diagnostics

Agent-Logs-Url: https://github.com/PowerShell/PowerShellEditorServices/sessions/19f2e25d-3a5e-4570-97bf-ce36493b9795

Co-authored-by: andyleejordan <2226434+andyleejordan@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: andyleejordan <2226434+andyleejordan@users.noreply.github.com>

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants