Skip to content

Optimize TestMethodRunner: fix double enumeration, reduce allocations#7541

Merged
Evangelink merged 5 commits intomainfrom
optimize-test-method-runner
Mar 15, 2026
Merged

Optimize TestMethodRunner: fix double enumeration, reduce allocations#7541
Evangelink merged 5 commits intomainfrom
optimize-test-method-runner

Conversation

@Evangelink
Copy link
Member

  • Fix double enumeration in TryExecuteFoldedDataDrivenTestsAsync by materializing GetData() into an IReadOnlyList once instead of calling .Any() then re-enumerating
  • Remove unnecessary List allocation in UpdateResultsWithParentInfo by mutating in place and returning void
  • Cache result[result.Length - 1] into a local in ExecuteAsync catch block
  • Skip redundant first-element self-comparison in GetAggregateOutcome
  • Use Stopwatch.StartNew() in ExecuteTestFromDataSourceAttributeAsync

- Fix double enumeration in TryExecuteFoldedDataDrivenTestsAsync by
  materializing GetData() into an IReadOnlyList once instead of calling
  .Any() then re-enumerating
- Remove unnecessary List allocation in UpdateResultsWithParentInfo by
  mutating in place and returning void
- Cache result[result.Length - 1] into a local in ExecuteAsync catch block
- Skip redundant first-element self-comparison in GetAggregateOutcome
- Use Stopwatch.StartNew() in ExecuteTestFromDataSourceAttributeAsync
Copilot AI review requested due to automatic review settings March 12, 2026 21:09
@Evangelink Evangelink enabled auto-merge March 12, 2026 21:09
- Hoist loop-invariant 'testContext as TestContextImplementation' cast
  above the foreach loop in RunAssemblyCleanupAsync
- Remove unnecessary nested scope in IsTestMethodRunnable
- Use 'is null' pattern instead of '== null'
- Simplify ignore message logic by inlining into the conditional block
Copy link
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 optimizes TestMethodRunner execution paths in the MSTest adapter by reducing redundant work (enumerations/allocations) during data-driven execution and result aggregation.

Changes:

  • Materialize folded data-driven GetData() results once to avoid double enumeration and enable Count checks.
  • Remove an unnecessary list allocation by mutating results in place in UpdateResultsWithParentInfo.
  • Minor micro-optimizations (cache last result in exception path, skip redundant first element in aggregate outcome, use Stopwatch.StartNew()).

You can also share your feedback on Copilot code review. Take the survey.

- Resolve merge conflict in TestMethodRunner.cs (take main's simplified catch block)
- Fix GetData() double-call: enumerate once with bool flag instead of materializing to list
- Fix UpdateResultsWithParentInfo XML doc to match actual behavior
- Preserve ignore message behavior in UnitTestRunner.cs (add shouldIgnoreMethod guard)
Copilot AI review requested due to automatic review settings March 14, 2026 10:39
Copy link
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

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.


You can also share your feedback on Copilot code review. Take the survey.

…path

- Update comment in TestMethodRunner to reference AssemblyEnumerator.TryUnfoldITestDataSource
- Update comment in AssemblyEnumerator to reference TestMethodRunner.TryExecuteFoldedDataDrivenTestsAsync
- Apply same single-enumeration pattern in AssemblyEnumerator.TryUnfoldITestDataSource to avoid double-enumerating GetData()
@Evangelink Evangelink disabled auto-merge March 15, 2026 06:04
@Evangelink Evangelink enabled auto-merge March 15, 2026 06:04
@Evangelink Evangelink merged commit 365f330 into main Mar 15, 2026
10 checks passed
@Evangelink Evangelink deleted the optimize-test-method-runner branch March 15, 2026 06:06
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.

3 participants