Skip to content

Add 20us tolerance to TestTotalProcessorTimeMacOs#116547

Merged
jozkee merged 2 commits intodotnet:mainfrom
jozkee:ProcessorTimeMacOs
Oct 21, 2025
Merged

Add 20us tolerance to TestTotalProcessorTimeMacOs#116547
jozkee merged 2 commits intodotnet:mainfrom
jozkee:ProcessorTimeMacOs

Conversation

@jozkee
Copy link
Member

@jozkee jozkee commented Jun 11, 2025 •
edited
Loading

Found on a release/9.0 build https://dev.azure.com/dnceng-public/public/_build/results?buildId=1064508

System.Diagnostics.Tests.ProcessTests.TestTotalProcessorTimeMacOs [FAIL]
  Time 'user' returned by managed API (00:00:01.4886940) should be greated or equal to the time returned by native API (00:00:01.4886980).
  Stack Trace:
    /_/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs(881,0): at System.Diagnostics.Tests.ProcessTests.<TestTotalProcessorTimeMacOs>g__AssertTime|229_0(TimeSpan managed, TimeSpan native, String label)
    /_/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs(870,0): at System.Diagnostics.Tests.ProcessTests.TestTotalProcessorTimeMacOs()
       at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
    /_/src/libraries/System.Private.CoreLib/src/System/Reflection/MethodBaseInvoker.cs(57,0): at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)

there was a 4 microsecond difference between the time calculated using macOS' proc_pid_rusage and mach_timebase_info and Process.UserProcessorTime.

I want to believe this was caused by rounding and/or precision differences between the macOS specific methods the test uses vs getrusage, used by the product.

Running the test locally on M1, I see the difference in the assertion is 655us.

@jozkee jozkee requested review from a team, Copilot and ericstj June 11, 2025 17:37
@dotnet-policy-service
Copy link
Contributor

dotnet-policy-service bot commented Jun 11, 2025

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

Copilot AI reviewed Jun 11, 2025
View reviewed changes
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

The PR adds a microsecond-level tolerance to the macOS processor time test to account for small discrepancies between the managed and native APIs.

  • Introduces a ToleranceInMicroseconds constant and computes the time difference in microseconds
  • Updates the assertion to allow a ±20 μs deviation
  • Adjusts the error message to mention the tolerance
Comments suppressed due to low confidence (1)

src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs:882

  • TimeSpan does not have a TotalMicroseconds property. Consider computing microseconds via (managed - native).Ticks / 10 or (managed - native).TotalMilliseconds * 1000.
double differenceUs = (managed - native).TotalMicroseconds;

@jozkee jozkee added the test-bug Problem in test source code (most likely) label Jun 11, 2025
@build-analysis build-analysis bot mentioned this pull request Jun 11, 2025
Closed
3 tasks
@tarekgh
Copy link
Member

tarekgh commented Oct 20, 2025 •
edited
Loading 

   [Fact]
   [PlatformSpecific(TestPlatforms.OSX)]
   public unsafe void TestTotalProcessorTimeMacOs()
   {

Just to tell, we have

Environment.CpuUsage too. just FYI if you want to double check against it too. not required though.

https://source.dot.net/#System.Private.CoreLib/src/libraries/System.Private.CoreLib/src/System/Environment.UnixOrBrowser.cs,82

Refers to: src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs:857 in 171b2cd. [](commit_id = 171b2cd, deletion_comment = False)

@jozkee jozkee enabled auto-merge (squash) October 20, 2025 21:08
@jozkee
Copy link
Member Author

jozkee commented Oct 21, 2025

/ba-g System.Security.Cryptography.Tests DeadLetter'd

@jozkee jozkee merged commit d852944 into dotnet:main Oct 21, 2025
83 of 85 checks passed
@jozkee jozkee deleted the ProcessorTimeMacOs branch October 21, 2025 15:16
@dotnet-maestro dotnet-maestro bot mentioned this pull request Oct 21, 2025
Merged
@github-actions github-actions bot locked and limited conversation to collaborators Nov 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jeffhandley jeffhandley jeffhandley left review comments

@tarekgh tarekgh tarekgh approved these changes

Copilot code review Copilot Copilot left review comments

@ericstj ericstj Awaiting requested review from ericstj

Assignees

@jozkee jozkee

Labels

area-System.Diagnostics.Process test-bug Problem in test source code (most likely)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jozkee @tarekgh @jeffhandley