Skip to content

Enable parallelization for blame data collector tests#15552

Merged
Evangelink merged 1 commit into
microsoft:mainfrom
nohwnd:investigate-blame-parallel
Mar 24, 2026
Merged

Enable parallelization for blame data collector tests#15552
Evangelink merged 1 commit into
microsoft:mainfrom
nohwnd:investigate-blame-parallel

Conversation

@nohwnd
Copy link
Copy Markdown
Member

@nohwnd nohwnd commented Mar 23, 2026

Summary

Move [DoNotParallelize] from class level to only the BlameDataCollectorAeDebuggerShouldCollectDump test method.

Investigation (Fixes #15530)

PR #15526 added [DoNotParallelize] to the entire BlameDataCollectorTests class with the comment "Blame tests collect crash/hang dumps from child processes and race when run in parallel." This is overly broad.

Why most blame tests CAN parallelize

Each blame test method:

  • Creates a unique TempDirectory (via RandomId.Next())
  • Launches its own vstest.console process with /Blame options
  • Each vstest.console creates its own testhost process
  • Procdump is attached per-process with unique file names ({processName}_{processId}_{timestamp})
  • Named events use unique process IDs (ProcDump-{targetProcessId})
  • PROCDUMP_PATH is passed as a per-process environment variable
  • BlameCollector uses only instance fields — no static mutable state

There is no shared state between parallel runs of these tests.

Why one test CANNOT parallelize

BlameDataCollectorAeDebuggerShouldCollectDump modifies machine-wide registry state by installing/uninstalling procdump as the Windows postmortem debugger (HKLM\SOFTWARE\Microsoft\Windows NT\CurrentVersion\AeDebug). Multiple parallel executions would race on this global registry key.

Change

  • Removed class-level [DoNotParallelize]
  • Added method-level [DoNotParallelize] only to BlameDataCollectorAeDebuggerShouldCollectDump
  • All 88 blame unit tests pass
  • Integration test project builds successfully

Move [DoNotParallelize] from class level to only the
BlameDataCollectorAeDebuggerShouldCollectDump test method, which is the
only test that modifies machine-wide state (HKLM registry via procdump
AeDebugger install/uninstall).

The other 10 blame tests use fully isolated state:
- Unique TempDirectory per test (RandomId)
- Separate vstest.console and testhost processes
- Per-process procdump attachment with unique file names
- Named events scoped by process ID

Fixes microsoft#15530

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 23, 2026 19:24
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 narrows test non-parallelization scope for blame data collector integration tests by moving [DoNotParallelize] from the entire BlameDataCollectorTests class to only the one test that mutates machine-wide AeDebug registry state.

Changes:

  • Removed class-level [DoNotParallelize] from BlameDataCollectorTests
  • Added method-level [DoNotParallelize] to BlameDataCollectorAeDebuggerShouldCollectDump
  • Updated comments to document the rationale for the remaining non-parallelized test

@nohwnd nohwnd added the 🚢 Ship it! Add to PRs where owner approves automated PR, but cannot approve because they "wrote it". label Mar 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🚢 Ship it! Add to PRs where owner approves automated PR, but cannot approve because they "wrote it".

Projects

None yet

Development

Successfully merging this pull request may close these issues.

blame tests cannot parallelize?

3 participants