Skip to content

Conversation

@MichalStrehovsky
Copy link
Member

Pull request #111850 improved reliability of this test for a bit but after we upgraded Helix machines to the new macOS, this test started very reliably fail with native AOT. My theory is that copying the binary around makes macOS treat the binary as a new thing that needs to be checked for signing/malware/etc and this now takes much longer than it used to. So stop copying the binary.

Pull request dotnet#111850 improved reliability of this test for a bit but after we upgraded Helix machines to the new macOS, this test started very reliably fail with native AOT. My theory is that copying the binary around makes macOS treat the binary as a new thing that needs to be checked for signing/malware/etc and this takes a long time. So stop copying the binary.
Copilot AI review requested due to automatic review settings March 14, 2025 05:27
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 improves the reliability of TestLongProcessIsWorking by eliminating the binary copying step, which appears to cause delays on macOS due to code signing and malware checks.

  • Removed conditional branch that distinguishes Linux from other platforms
  • Always creates a wrapped sleep script using GetTestFilePath() instead of copying the sleep binary
Comments suppressed due to low confidence (1)

src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs:2274

  • Removing the OS check may cause unintended behavior on platforms other than macOS. Consider adding a conditional check (e.g., OperatingSystem.IsMacOS()) to ensure this new behavior is only applied where needed.
string sleepPath = GetTestFilePath();

@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process
See info in area-owners.md if you want to be subscribed.

@MichalStrehovsky
Copy link
Member Author

/azp run runtime-nativeaot-outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

LGTM assuming the CI passes

@MichalStrehovsky
Copy link
Member Author

LGTM assuming the CI passes

Nope, going to block it instead at #113600.

@MichalStrehovsky MichalStrehovsky deleted the patch-1 branch March 17, 2025 06:28
@github-actions github-actions bot locked and limited conversation to collaborators Apr 16, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants