Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use temp directory as TestExecutionDirectory in RunTestsOnHelix.sh #45022

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

akoeplinger
Copy link
Member

@akoeplinger akoeplinger commented Nov 21, 2024

I looked into using the temp directory on Unix like we do on Windows and it uncovered two issues (#3923 was an earlier report)

  1. Helix sets $TMPDIR to /tmp/helix but on macOS /tmp is a symlink to /private/tmp and a bunch of tests then fail since parts of the sdk see /tmp and other parts see the resolved /private/tmp path and checks for path equality and other similar logic fails (see also .NET SDK fails to resolve transitive references in standard macOS $TMPDIR and other symlinked directories #37687)
    -> this was relatively easy to solve by making sure we use the resolved path as the TestExecutionDirectory. Ideally we'd solve the underlying symlink equality issue but that's a bigger change.

  2. the test GivenWorkloadUpdateAcrossFeatureBandsItUpdatesPacks started failing in CI but only on Linux. I tried reproducing it locally but it always passed. Then I stepped through it under the debugger and noticed that I got this exception here:

    System.IO.IOException : The process cannot access the file '/Users/alexander/dev/sdk/artifacts/tmp/Debug/GivenWorkload---B940FC6C/dotnet/metadata/workloads/InstalledPacks/v1/mock-pack-2/2.0.0/5.0.100' because it is being used by another process.

    ... but we're swallowing the exception so the test accidentally passed because the workload GC was aborted.

    I tracked it down to us not closing the file stream created by File.Create() during the test, once I fixed that the test now consistently failed everywhere (it asserts that the file should exist but it gets deleted during workload GC). This was fixed by mocking up a 5.0.1xx SDK install so that the installation records for that feature band won't be deleted.

We're now correctly closing all file streams created by File.Create() in the codebase.

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Infrastructure untriaged Request triage from a team member labels Nov 21, 2024
@dsplaisted
Copy link
Member

@akoeplinger @joeloff I pushed a commit which should fix the workload test failure.

@akoeplinger
Copy link
Member Author

@dsplaisted thanks, I updated the PR description to capture the root cause.

@akoeplinger akoeplinger marked this pull request as ready for review November 27, 2024 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Infrastructure untriaged Request triage from a team member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants