Skip to content

Fix flaky tests: remove process-wide Directory.SetCurrentDirectory and anchor cwd assertions to absolute paths#7487

Merged
jozkee merged 3 commits intomainfrom
copilot/fix-flaky-save-to-async-test
Apr 29, 2026
Merged

Fix flaky tests: remove process-wide Directory.SetCurrentDirectory and anchor cwd assertions to absolute paths#7487
jozkee merged 3 commits intomainfrom
copilot/fix-flaky-save-to-async-test

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 24, 2026

DownloadToAsync_EmptyDestinationPath_UsesCurrentDirectory called Directory.SetCurrentDirectory, mutating process-wide state. xUnit runs test classes in parallel by default, so this raced with SaveToAsync_WithJustFilename_SavesInCurrentDirectory and SaveToAsync_WithEmptyPath_UsesCurrentDirectoryAndContentName, both of which relied on File.Exists(relativePath) — resolving silently against whatever cwd happened to be at check time.

Changes

  • HostedFileClientExtensionsTests.csDownloadToAsync_EmptyDestinationPath_UsesCurrentDirectory

    • Removed Directory.SetCurrentDirectory entirely; no temp directory created or deleted
    • Uses a GUID-based filename to avoid collisions
    • Captures Directory.GetCurrentDirectory() before the call; asserts file existence and content via Path.Combine(cwdBefore, savedPath)
    • finally deletes only the single written file by absolute path
  • DataContentTests.csSaveToAsync_WithJustFilename_SavesInCurrentDirectory and SaveToAsync_WithEmptyPath_UsesCurrentDirectoryAndContentName

    • Both tests now capture cwd before the call and assert against Path.Combine(cwdBefore, filename) rather than a bare relative path, making the assertion independent of cwd at check time
    • Cleanup in finally also uses the absolute path
// Before — racy: File.Exists resolves against whatever cwd is at check time
savedPath = await content.SaveToAsync(filename);
Assert.True(File.Exists(savedPath));  // savedPath is just "test_<guid>.json"

// After — anchored to cwd captured before the call
string cwdBefore = Directory.GetCurrentDirectory();
string expectedAbsolute = Path.Combine(cwdBefore, filename);
savedPath = await content.SaveToAsync(filename);
Assert.True(File.Exists(expectedAbsolute));

Directory.SetCurrentDirectory is no longer called anywhere in the test assembly.

Original prompt

Problem

The test SaveToAsync_WithJustFilename_SavesInCurrentDirectory in test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/Contents/DataContentTests.cs is flaky in CI. Example failure:

Assert.True() Failure
Expected: True
Actual:   False
   at Microsoft.Extensions.AI.DataContentTests.SaveToAsync_WithJustFilename_SavesInCurrentDirectory() in DataContentTests.cs:line 609

Root cause

The test DownloadToAsync_EmptyDestinationPath_UsesCurrentDirectory in test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/Files/HostedFileClientExtensionsTests.cs calls Directory.SetCurrentDirectory(tempDir), which mutates process-wide state. xUnit runs different test classes in parallel by default, so when DataContentTests and HostedFileClientExtensionsTests interleave:

  1. SaveToAsync_WithJustFilename_SavesInCurrentDirectory writes test_<guid>.json to cwd (relative path).
  2. DownloadToAsync_EmptyDestinationPath_UsesCurrentDirectory switches cwd to its temp dir, then Directory.Delete(tempDir, recursive: true) wipes the just-written file (if cwd happened to be that temp dir at write time) or moves cwd so File.Exists(savedPath) in the other test resolves a relative path against the wrong directory.

Either way File.Exists(savedPath) returns false non-deterministically.

Additionally, SaveToAsync_WithJustFilename_SavesInCurrentDirectory does not actually verify the file landed in cwd — it just checks File.Exists(savedPath) on a relative path, which is an implicit, time-of-check resolution against whatever cwd happens to be, not a real "is in cwd" assertion.

Fix

Make two changes:

1. Remove Directory.SetCurrentDirectory from DownloadToAsync_EmptyDestinationPath_UsesCurrentDirectory

Stop mutating process-wide cwd. Instead, capture cwd before the call and assert the file exists at Path.Combine(cwdBefore, savedPath). This preserves the "cwd-at-call-time" semantic check without racing with other tests. Use a GUID-based filename to avoid collisions, and delete just that file in finally.

File: test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/Files/HostedFileClientExtensionsTests.cs

Replace the existing test body with something like:

[Fact]
public async Task DownloadToAsync_EmptyDestinationPath_UsesCurrentDirectory()
{
    var data = new byte[] { 42, 43 };
    var fileName = $"output_{Guid.NewGuid():N}.bin";

    using var client = new TestHostedFileClient
    {
        DownloadAsyncCallback = (fileId, options, ct) =>
            Task.FromResult<HostedFileDownloadStream>(new TestHostedFileDownloadStream(data, fileName: fileName))
    };

    // Capture cwd at call time so the assertion doesn't depend on cwd at check time.
    string cwdBefore = Directory.GetCurrentDirectory();
    string? savedPath = null;
    try
    {
        savedPath = await client.DownloadToAsync("file-cwd", string.Empty);

        // Empty destination path => file is written to cwd; returned path is just the filename (relative).
        Assert.Equal(fileName, savedPath);
        Assert.False(Path.IsPathRooted(savedPath));

        string expectedAbsolute = Path.Combine(cwdBefore, savedPath);
        Assert.True(File.Exists(expectedAbsolute));
        Assert.Equal(data, await File.ReadAllBytesAsync(expectedAbsolute));
    }
    finally
    {
        if (savedPath is not null)
        {
            string absolute = Path.Combine(cwdBefore, savedPath);
            if (File.Exists(absolute)) File.Delete(absolute);
        }
    }
}

Do not call Directory.SetCurrentDirectory anywhere in this test (or anywhere else in the test assembly). No tempDir creation/deletion is needed for this test anymore.

2. Tighten SaveToAsync_WithJustFilename_SavesInCurrentDirectory to actually check cwd

File: test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/Contents/DataContentTests.cs

Replace the body of SaveToAsync_WithJustFilename_SavesInCurrentDirectory with a version that captures cwd up front and asserts the file exists at the absolute path resolved from that captured cwd:

[Fact]
public async Task SaveToAsync_WithJustFilename_SavesInCurrentDirectory()
{
    // Test that providing just a filename (no directory) saves to current directory
    byte[] testData = [1, 2, 3];
    DataContent content = new(testData, "application/json");

    string filename = $"test_{Guid.NewGuid():N}.json";
    string cwdBefore = Directory.GetCurrentDirectory();
    string expectedAbsolute = Path.Combine(cwdBefore, filename);
    string? savedPath = null;

    try
    {
        savedPath = await content.SaveToAsync(filename);

        // The returned path should be just the filename (relative to cwd).
        Assert.Equal(filename, Path.GetFileName(savedPath));

        // Verify the file actually landed in cwd-at-call-time using an absolute path,
        // so the assertion is independent of cwd at check time.
        Assert.True(File.Exists(expectedAb...

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

*This pull request was created from Copilot chat.*
>

Copilot AI changed the title [WIP] Fix flaky test SaveToAsync_WithJustFilename_SavesInCurrentDirectory Fix flaky tests: remove process-wide Directory.SetCurrentDirectory and anchor cwd assertions to absolute paths Apr 24, 2026
Copilot AI requested a review from jozkee April 24, 2026 15:12
Co-authored-by: David Cantú <dacantu@microsoft.com>
@jozkee jozkee added area-ai Microsoft.Extensions.AI libraries and removed area-telemetry labels Apr 29, 2026
@jozkee jozkee marked this pull request as ready for review April 29, 2026 18:06
@jozkee jozkee requested a review from a team as a code owner April 29, 2026 18:06
Copilot AI review requested due to automatic review settings April 29, 2026 18:06
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

Removes process-wide current-directory mutation from the AI Abstractions test suite and makes “saves to current directory” assertions deterministic under xUnit’s default parallel execution.

Changes:

  • Eliminated Directory.SetCurrentDirectory usage from DownloadToAsync_EmptyDestinationPath_UsesCurrentDirectory.
  • Anchored current-directory file assertions/cleanup to cwdBefore + filename (absolute path) rather than relying on relative-path resolution at assertion time.
  • Updated DataContent.SaveToAsync tests to assert and clean up via absolute paths derived from the captured cwd.

Reviewed changes

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

File Description
test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/Files/HostedFileClientExtensionsTests.cs Removes process-wide cwd mutation; asserts download output via absolute path based on captured cwd.
test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/Contents/DataContentTests.cs Reworks “current directory” save assertions/cleanup to use absolute paths based on captured cwd.

Copy link
Copy Markdown
Member

@tarekgh tarekgh left a comment

Choose a reason for hiding this comment

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

LGTM

@jozkee jozkee enabled auto-merge (squash) April 29, 2026 19:38
@jozkee jozkee merged commit 7171956 into main Apr 29, 2026
10 checks passed
@jozkee jozkee deleted the copilot/fix-flaky-save-to-async-test branch April 29, 2026 19:47
jeffhandley pushed a commit that referenced this pull request May 1, 2026
…and anchor cwd assertions to absolute paths (#7487)

* Initial plan

* Fix flaky tests: remove Directory.SetCurrentDirectory and harden cwd assertions

Agent-Logs-Url: https://github.com/dotnet/extensions/sessions/d232918b-7fce-4025-b36c-84d01c01aaeb

Co-authored-by: jozkee <16040868+jozkee@users.noreply.github.com>

* Apply suggestions from code review

Co-authored-by: David Cantú <dacantu@microsoft.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: jozkee <16040868+jozkee@users.noreply.github.com>
Co-authored-by: David Cantú <dacantu@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-ai Microsoft.Extensions.AI libraries

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants