Add a workaround to force VS Code to trigger onDidCloseTextDocument#5886
Add a workaround to force VS Code to trigger onDidCloseTextDocument#5886
Conversation
We have code that handles closing and opening of text documents, but VS Code only actually closes them after 3 mins. Rather than have tests waiting for 3 mins or risking behaving differently to when files are really closed, this spawns dummy files that forces VS Code to clean up in places where we want to be sure that event fires.
DanTup
added
the
is testing
|
@codex review /gemini review |
|
Hello DanTup, I've received your request for a code review via Please note that Thank you! |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5886 +/- ##
=======================================
Coverage 67.37% 67.37%
=======================================
Files 168 168
Lines 12899 12899
Branches 2556 2556
=======================================
Hits 8691 8691
+ Misses 3757 3754 -3
- Partials 451 454 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR introduces a workaround to force VS Code to trigger onDidCloseTextDocument events immediately during testing, rather than waiting up to 3 minutes for VS Code's natural cleanup process. The solution involves opening dummy untitled documents to fill VS Code's text model buffer, forcing it to clean up previously closed documents.
Changes:
- Implemented
forceDocumentCloseEvents()function that opens up to 100 dummy files to trigger document cleanup - Refactored
closeFile()to use the new workaround and ensure close events fire immediately - Removed
waitUntilAllTextDocumentsAreClosed()and related slow test configurations - Added comprehensive test coverage for the new functionality
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/test/helpers.ts | Adds forceDocumentCloseEvents() and closeTabs() functions, refactors closeFile(), removes waitUntilAllTextDocumentsAreClosed() |
| src/test/dart/utils/helpers.test.ts | New test file verifying that closeFile() properly triggers close events through 10 iterations |
| src/test/dart/refactors/renames.test.ts | Updates to use new workaround, removes 3-minute timeout and slow test skip logic |
| src/test/dart/file_tracking.test.ts | Updates subscription tests to use new workaround, removes slow test configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](proxy.php?url=https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9a0e8d9540
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
DanTup
force-pushed
the
force-close-hack
branch
from
18280ac to
5ff49c3
Compare
DanTup
force-pushed
the
force-close-hack
branch
from
5ff49c3 to
9b24e5c
Compare
|
@codex review /gemini review |
![gemini-code-assist[bot]](proxy.php?url=https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a clever workaround to force VS Code to fire onDidCloseTextDocument events during tests, which should significantly improve test reliability and speed by removing the need for long waits. The new forceDocumentCloseEvents helper is a great addition. I've provided a couple of suggestions for src/test/helpers.ts to enhance the maintainability of the new code by addressing a magic number and clarifying some logic. Overall, this is a valuable improvement for the test suite.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 53b52a5e2c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
documents are forceable closed and reopened when the language is changed tho that prob doesn't work well enough for you |
|
@RedCMD thanks! I did actually see that yesterday, but also wasn't sure it would help here because some tests are trying to check things for closed files, and I'm trying really to mimic as close to the real behaviour as possible. Honestly I don't understand why we wan't change the 3min timeout or have some command to force this (particularly for testing). I did also consider (and started implementing) just simulating the close/open events - however it got really messy (for example VS Code is still sending Change events for files modified on disk when the file is closed-but-not-really-closed-yet, so they would need suppressing too). Unfortuantely all the options are bad, but this one seemd like the least bad (it's isolated to the internals of a single function) and in my testing was also reliable. Unfortunately it's not fast though, opening and closing 60 documents takes quite a few seconds 😞 |
We have code that handles closing and opening of text documents, but VS Code only actually closes them after 3 mins. Rather than have tests waiting for 3 mins or risking behaving differently to when files are really closed, this spawns dummy files that forces VS Code to clean up in places where we want to be sure that event fires.