Skip to content

Explicitly dispose summaryWriter before reading file in truncation tests#76

Merged
Tyrrrz merged 2 commits intoprimefrom
copilot/investigate-windows-build-failure
May 1, 2026
Merged

Explicitly dispose summaryWriter before reading file in truncation tests#76
Tyrrrz merged 2 commits intoprimefrom
copilot/investigate-windows-build-failure

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 1, 2026

In the "nearly full summary file" truncation tests, the summaryWriter (backed by a file stream opened for writing) was left open when File.ReadAllBytes was called. On Windows, opening a second handle for reading conflicts with the existing write handle, causing the test to fail.

The fix adds an explicit await summaryWriter.DisposeAsync() call immediately after FlushAsync() and before the file read in both affected tests:

  • VsTestSummarySpecs.csI_can_try_to_use_the_logger_to_produce_a_summary_when_the_output_file_is_nearly_full_and_get_a_truncated_summary
  • MtpSummarySpecs.cs — same test in the MTP variant

Closing the write handle first means File.ReadAllBytes can always open the file for reading without a sharing-mode conflict, on both Windows and Linux.

Copilot finished work on behalf of Tyrrrz May 1, 2026 13:56
Copilot AI requested a review from Tyrrrz May 1, 2026 13:56
@Tyrrrz Tyrrrz added the bug Something isn't working label May 1, 2026
@Tyrrrz Tyrrrz marked this pull request as ready for review May 1, 2026 20:26
Copilot AI review requested due to automatic review settings May 1, 2026 20:26
@codecov
Copy link
Copy Markdown

codecov Bot commented May 1, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.22%. Comparing base (ccacd59) to head (6d846bc).
⚠️ Report is 1 commits behind head on prime.

Additional details and impacted files
@@           Coverage Diff           @@
##            prime      #76   +/-   ##
=======================================
  Coverage   85.22%   85.22%           
=======================================
  Files          19       19           
  Lines         758      758           
  Branches       85       85           
=======================================
  Hits          646      646           
  Misses         59       59           
  Partials       53       53           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Fixes Windows-only failures in the “nearly full summary file” truncation tests by ensuring the summary file’s write handle is closed before attempting to reopen the same file for reading.

Changes:

  • Explicitly disposes the file-backed summaryWriter after FlushAsync() and before reading bytes from the summary file in the VsTest truncation test.
  • Applies the same disposal fix to the MTP truncation test variant.

Reviewed changes

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

File Description
GitHubActionsTestLogger.Tests/VsTestSummarySpecs.cs Disposes the file-backed summary writer before reading the summary file to avoid sharing conflicts on Windows.
GitHubActionsTestLogger.Tests/MtpSummarySpecs.cs Mirrors the same early-dispose behavior for the MTP variant of the truncation test.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread GitHubActionsTestLogger.Tests/VsTestSummarySpecs.cs
Comment thread GitHubActionsTestLogger.Tests/MtpSummarySpecs.cs
This was referenced May 5, 2026
github-actions Bot pushed a commit to saan800/github that referenced this pull request May 6, 2026
…#249)

Updated
[GitHubActionsTestLogger](https://github.com/Tyrrrz/GitHubActionsTestLogger)
from 3.0.2 to 3.0.4.

<details>
<summary>Release notes</summary>

_Sourced from [GitHubActionsTestLogger's
releases](https://github.com/Tyrrrz/GitHubActionsTestLogger/releases)._

## 3.0.4

<!-- Release notes generated using configuration in .github/release.yml
at 3.0.4 -->

## What's Changed
### Enhancements
* Add PowerKit (prerelease), update PolyShim and CSharpier, remove
duplicate utilities by @​Copilot in
Tyrrrz/GitHubActionsTestLogger#72
### Bugs
* Explicitly dispose `summaryWriter` before reading file in truncation
tests by @​Copilot in
Tyrrrz/GitHubActionsTestLogger#76
* Fix GITHUB_STEP_SUMMARY size limit exceeded with non-ASCII characters
by @​Copilot in
Tyrrrz/GitHubActionsTestLogger#75


**Full Changelog**:
Tyrrrz/GitHubActionsTestLogger@3.0.3...3.0.4

## 3.0.3

## What's Changed
* Truncate or omit step summary when approaching GitHub's 1 MiB limit by
@​Copilot in Tyrrrz/GitHubActionsTestLogger#68
* Pin Microsoft.Testing.Platform reference to 2.0.0 by @​Copilot in
Tyrrrz/GitHubActionsTestLogger#70


**Full Changelog**:
Tyrrrz/GitHubActionsTestLogger@3.0.2...3.0.3

Commits viewable in [compare
view](Tyrrrz/GitHubActionsTestLogger@3.0.2...3.0.4).
</details>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants