Skip to content

Cleaning up noise from test run output - #22449

Open
Benjin wants to merge 14 commits into
mainfrom
dev/benjin/cleanupTestOutput
Open

Cleaning up noise from test run output#22449
Benjin wants to merge 14 commits into
mainfrom
dev/benjin/cleanupTestOutput

Conversation

@Benjin

@Benjin Benjin commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Description

Cleans up several classes of test output pollution:

  • missing stubs for side-effect calls that weren't defined
  • (intentionally) unawaited promises that would complete after the test had already finished and cleaned up
  • slow docker unit tests due to hardcoded timer instead of stub
  • missing stubs for tests that incidentally involve calls to read connections/groups from config

Code Changes Checklist

  • New or updated unit tests added
  • All existing tests pass (npm run test)
  • Code follows contributing guidelines
  • Telemetry/logging updated if relevant
  • No regressions or UX breakage

Reviewers: Please read our reviewer guidelines

Benjin and others added 13 commits June 26, 2026 21:41
Webview controllers reject an internal _webviewReady deferred when they are
disposed before the webview signals readiness. Because that rejection settles
asynchronously, Mocha reports it as a stray 'rejected promise not handled
within 1 second' against an unrelated test.

Add an observeWebviewReady test helper that attaches a no-op catch to the
controller's _webviewReady promise, and call it wherever tests construct (or
internally dispose) webview controllers. Test-only change; no product code is
modified.
WebviewPanelController.dispose rejects the base controller's _webviewReady
deferred, producing an unhandled-rejection warning ~1s after the test ends.
Attach observeWebviewReady in the panel controller test factory so the
rejection is observed. Test-only change; no product code is modified.
…oise

After a successful save, ResultsSerializer calls showInformationMessage().then()
and shouldOpenSavedFile() (config.get) on its VscodeWrapper, and openSavedFile()
chains openTextDocument().then()/showTextDocument(). Tests that drove these
fire-and-forget success paths left showInformationMessage, openTextDocument,
showTextDocument and getConfiguration().get unstubbed, so the chains rejected
~1s later as unhandled rejections. Add the missing stubs (including a get() on
the custom-config wrapper). Test-only change; no product code is modified.
… noise

The ConnectionDialogWebviewController constructor kicks off a fire-and-forget
loadVscodeEntraDataAsync() when VS Code account mode is enabled. After a test
disposes the controller, that background work calls updateState ->
sendNotification on the disposed controller, surfacing as an unhandled
'Cannot send notification on disposed controller' rejection ~1s later.

Stub the method on the prototype in setup and restore it in the two tests that
exercise the real implementation. Test-only change; no product code is modified.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR reduces unit test “noise” (unhandled promise rejections, missing stubs, and avoidable real-time delays) to make test runs cleaner and faster, especially around webview-controller disposal and connection-store initialization side effects.

Changes:

  • Added a unit-test helper to observe (and handle) internal “webview ready” promise rejections when controllers are disposed before becoming ready, and applied it across webview-related tests.
  • Updated several test suites to await controller/service initialization and stub previously-unstubbed ConnectionStore reads (connections/groups) to prevent incidental side-effect calls.
  • Improved docker-related unit test performance by stubbing spawnSync (to avoid fix-path shell-outs) and using fake timers instead of real-time polling delays.

Reviewed changes

Copilot reviewed 15 out of 35 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
localization/xliff/sql-database-projects.zh-Hans.xlf Auto-generated localization file change (ignored for code review).
extensions/mssql/test/unit/webviewPanelController.test.ts Uses new webview-ready observer helper to avoid unhandled rejections on disposal.
extensions/mssql/test/unit/webviewBaseController.test.ts Uses new webview-ready observer helper to avoid unhandled rejections on disposal.
extensions/mssql/test/unit/utils.ts Adds observeWebviewReady helper for handling disposed-before-ready promise rejection noise in tests.
extensions/mssql/test/unit/userSurvey.test.ts Observes webview readiness to prevent unhandled rejection noise during disposal.
extensions/mssql/test/unit/shortcutsConfigurationWebviewController.test.ts Observes webview readiness to prevent unhandled rejection noise during disposal.
extensions/mssql/test/unit/schemaDesignerWebviewManager.test.ts Observes webview readiness before disposing designers to prevent unhandled rejection noise.
extensions/mssql/test/unit/schemaDesignerWebviewController.test.ts Observes webview readiness to prevent unhandled rejection noise during disposal.
extensions/mssql/test/unit/profiler/profilerWebviewController.test.ts Observes webview readiness for created profiler webviews to prevent unhandled rejection noise.
extensions/mssql/test/unit/profiler/profilerController.test.ts Wraps internal profiler webview disposal to observe readiness and prevent unhandled rejection noise.
extensions/mssql/test/unit/perFileConnection.test.ts Awaits ConnectionManager.initialized and stubs connection store initialization to avoid side-effect noise.
extensions/mssql/test/unit/objectExplorerService.test.ts Adds missing connection/group stubs and awaits OE initialization to avoid background side effects polluting tests.
extensions/mssql/test/unit/dockerUtilities.test.ts Stubs spawnSync and uses fake timers to avoid slow real-time polling and shell-outs in unit tests.
extensions/mssql/test/unit/connectionManager.test.ts Awaits initialization and stubs connection-store initialization/reads to reduce incidental side effects and noise.
extensions/mssql/test/unit/connectionDialogWebviewController.test.ts Stubs fire-and-forget Entra data load and observes webview readiness to avoid disposed-controller rejections.
extensions/mssql/test/unit/addFirewallRuleWebviewController.test.ts Observes webview readiness to prevent unhandled rejection noise during disposal.

Comment on lines +368 to +373
export function observeWebviewReady(controller: unknown): void {
const ready = (controller as { _webviewReady?: { promise?: Promise<void> } })?._webviewReady;
void ready?.promise?.catch(() => {
/* Swallow the disposed-before-ready rejection in tests. */
});
}
Comment on lines +898 to +902
const initializedDeferred = new Deferred<void>();
initializedDeferred.resolve();
Object.defineProperty(mockConnectionStore, "initialized", {
get: () => initializedDeferred,
});
Comment on lines +1114 to +1118
const initializedDeferred = new Deferred<void>();
initializedDeferred.resolve();
Object.defineProperty(mockConnectionStore, "initialized", {
get: () => initializedDeferred,
});
Copilot AI review requested due to automatic review settings July 8, 2026 05:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 16 out of 36 changed files in this pull request and generated 3 comments.

Comment on lines +368 to +373
export function observeWebviewReady(controller: unknown): void {
const ready = (controller as { _webviewReady?: { promise?: Promise<void> } })?._webviewReady;
void ready?.promise?.catch(() => {
/* Swallow the disposed-before-ready rejection in tests. */
});
}
Comment on lines +898 to +902
const initializedDeferred = new Deferred<void>();
initializedDeferred.resolve();
Object.defineProperty(mockConnectionStore, "initialized", {
get: () => initializedDeferred,
});
Comment on lines +1113 to +1117
const initializedDeferred = new Deferred<void>();
initializedDeferred.resolve();
Object.defineProperty(mockConnectionStore, "initialized", {
get: () => initializedDeferred,
});
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown

PR Changes

Category Target Branch PR Branch Difference
vscode-mssql VSIX 79122 KB 79122 KB ⚪ 0 KB ( 0% )
sql-database-projects VSIX 6288 KB 6288 KB ⚪ 0 KB ( 0% )
data-workspace VSIX 535 KB 535 KB ⚪ 0 KB ( 0% )
keymap VSIX 7 KB 7 KB ⚪ 0 KB ( 0% )

@codecov-commenter

codecov-commenter commented Jul 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.63%. Comparing base (09f125f) to head (6bb4a3e).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #22449      +/-   ##
==========================================
- Coverage   75.63%   75.63%   -0.01%     
==========================================
  Files         412      412              
  Lines      129871   129886      +15     
  Branches     8307     8298       -9     
==========================================
+ Hits        98230    98239       +9     
- Misses      31641    31647       +6     
Flag Coverage Δ
data-workspace 77.10% <ø> (ø)
mssql 75.38% <100.00%> (-0.01%) ⬇️
sqlproj 78.02% <ø> (ø)

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

Files with missing lines Coverage Δ
...nectionconfig/connectionDialogWebviewController.ts 75.17% <100.00%> (-0.15%) ⬇️
extensions/mssql/test/unit/utils.ts 97.58% <100.00%> (+0.09%) ⬆️

... and 1 file with indirect coverage changes

🚀 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.

@aasimkhan30

Copy link
Copy Markdown
Contributor

@Benjin, can you configure the runner to not show a line entry for passed tests? Adds a lot of scrolling to the GitHub runs.

@aasimkhan30

Copy link
Copy Markdown
Contributor

Seems like line endings for loc files are messed up.

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.

4 participants