Skip to content

Give AttachProcessProvider suite the shared TEST_TIMEOUT - #1150

Open
TEJA P C (PCTEJA) wants to merge 1 commit into
microsoft:mainfrom
PCTEJA:fix-provider-test-timeout-1031
Open

TEJA P C (PCTEJA) wants to merge 1 commit into
microsoft:mainfrom
PCTEJA:fix-provider-test-timeout-1031

Conversation

@PCTEJA

Copy link
Copy Markdown

Summary

Fixes #1031.

The Attach to process - process provider suite intermittently fails on the first run: on a cold start, the first test can exceed Mocha's 2s default timeout (initial module loading). When it times out, its still-pending async work completes during the next test and records extra calls on that test's sinon stubs, failing it with errors like expected plainExec to be called once but it was called twice.

Change

Test-only, one file (src/test/unittest/attachQuickPick/provider.unit.test.ts): give the suite the repository's shared timeout instead of the 2s default —

suite('Attach to process - process provider', function () {
    this.timeout(TEST_TIMEOUT);
    ...

TEST_TIMEOUT (60s) is the existing shared constant in src/test/constants.ts, and this.timeout(TEST_TIMEOUT) is the pattern already used elsewhere in this repo's tests (e.g. reportIssueCommand.unit.test.ts). With the first test no longer timing out under cold-start load, its async work completes within its own test and cannot pollute the next test's stubs. No assertions, stubs, or production code change.

Testing

  • Baseline and patched runs of this suite in a real VS Code 1.146 test host: 13 passing both times on this (warm) machine — the flake requires cold-start slowness, so it does not reproduce locally; the change follows the mechanism and error signature documented in Race condition in provider.unit.test.ts due to slow initial execution #1031.
  • npm run compile-tests (tsc) passes; eslint and prettier --check pass on the changed file.
  • The patch was independently reviewed with Codex (GPT-6 Astra), which verified with an executable Mocha TDD probe that the suite-level timeout (60,000 ms) is inherited by all tests/hooks in the suite while the root default stays 2,000 ms, and confirmed the import path and that no other test behaviour changes.

This change was prepared with AI assistance (Muse, validated with Codex); I have reviewed every changed line and take responsibility for it.

On a cold start, the first test in this suite can exceed Mocha's 2s
default timeout (initial module loading). When it times out, its
still-pending async work completes during the next test and records
extra calls on that test's sinon stubs, failing it intermittently
(e.g. 'expected plainExec to be called once but it was called twice').

Use the repository's shared TEST_TIMEOUT (60s, src/test/constants.ts)
for the suite, following the existing this.timeout(TEST_TIMEOUT)
pattern used elsewhere in the test suite.

Fixes microsoft#1031
@PCTEJA

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

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.

Race condition in provider.unit.test.ts due to slow initial execution

1 participant