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

System.Diagnostics.Tests.InstanceDataTests.InstanceDataCollectionCollection_CopyTo failing in CI #68291

Closed
ViktorHofer opened this issue Apr 20, 2022 · 6 comments · Fixed by #70119
Assignees
Labels
area-System.Diagnostics.PerformanceCounter blocking-clean-ci Blocking PR or rolling runs of 'runtime' or 'runtime-extra-platforms'
Milestone

Comments

@ViktorHofer
Copy link
Member

ViktorHofer commented Apr 20, 2022

Configuration: net7.0-windows-Release-x86-CoreCLR_release-Windows.10.Amd64.Server2022.ES.Open

Build: https://dev.azure.com/dnceng/public/_build/results?buildId=1721549

    System.Diagnostics.Tests.InstanceDataTests.InstanceDataCollectionCollection_CopyTo [FAIL]
      Assert.True() Failure
      Expected: True
      Actual:   False
      Stack Trace:
        /_/src/libraries/System.Diagnostics.PerformanceCounter/tests/InstanceDataTests.cs(122,0): at System.Diagnostics.Tests.InstanceDataTests.InstanceDataCollectionCollection_CopyTo()
           at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
        /_/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.CoreCLR.cs(379,0): at System.Reflection.RuntimeMethodInfo.InvokeNonEmitUnsafe(Object obj, IntPtr* arguments, Span`1 argsForTemporaryMonoSupport, BindingFlags invokeAttr)

Informaci�n: Los contadores de rendimiento se instalaron correctamente en C:\h\w\B1040971\t\h2gyudok.4kw\provider.man.    System.Diagnostics.Tests.PerformanceDataTests.PerformanceCounter_PerformanceData [SKIP]
      Condition(s) not met: "IsRunnableEnvironment"

Informaci�n: Desinstalaci�n correcta de los contadores de rendimiento del archivo XML de definici�n de contadores C:\h\w\B1040971\t\h2gyudok.4kw\provider.man.  Finished:    System.Diagnostics.PerformanceCounter.Tests
=== TEST EXECUTION SUMMARY ===
   System.Diagnostics.PerformanceCounter.Tests  Total: 90, Errors: 0, Failed: 1, Skipped: 1, Time: 30,024s

Runfo Tracking Issue: system.diagnostics.tests.instancedatatests.instancedatacollectioncollection_copyto

Build Definition Kind Run Name Console Core Dump Test Results Run Client
1799703 runtime PR 70062 net7.0-windows-Release-x64-CoreCLR_checked-Windows.10.Amd64.Open console.log runclient.py
1757267 runtime PR 68993 net7.0-windows-Debug-x64-CoreCLR_release-Windows.11.Amd64.ClientPre.Open console.log runclient.py
1744574 runtime Rolling net7.0-windows-Release-x64-CoreCLR_release-Windows.11.Amd64.ClientPre.Open console.log runclient.py
1725393 runtime PR 67883 net48-windows-Release-x86-Windows.10.Amd64.Client21H1.Open console.log runclient.py

Build Result Summary

Day Hit Count Week Hit Count Month Hit Count
0 1 2
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Apr 20, 2022
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@ghost
Copy link

ghost commented Apr 20, 2022

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

Issue Details

Configuration: net7.0-windows-Release-x86-CoreCLR_release-Windows.10.Amd64.Server2022.ES.Open

Build: https://dev.azure.com/dnceng/public/_build/results?buildId=1721549

    System.Diagnostics.Tests.InstanceDataTests.InstanceDataCollectionCollection_CopyTo [FAIL]
      Assert.True() Failure
      Expected: True
      Actual:   False
      Stack Trace:
        /_/src/libraries/System.Diagnostics.PerformanceCounter/tests/InstanceDataTests.cs(122,0): at System.Diagnostics.Tests.InstanceDataTests.InstanceDataCollectionCollection_CopyTo()
           at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
        /_/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.CoreCLR.cs(379,0): at System.Reflection.RuntimeMethodInfo.InvokeNonEmitUnsafe(Object obj, IntPtr* arguments, Span`1 argsForTemporaryMonoSupport, BindingFlags invokeAttr)

Informaci�n: Los contadores de rendimiento se instalaron correctamente en C:\h\w\B1040971\t\h2gyudok.4kw\provider.man.    System.Diagnostics.Tests.PerformanceDataTests.PerformanceCounter_PerformanceData [SKIP]
      Condition(s) not met: "IsRunnableEnvironment"

Informaci�n: Desinstalaci�n correcta de los contadores de rendimiento del archivo XML de definici�n de contadores C:\h\w\B1040971\t\h2gyudok.4kw\provider.man.  Finished:    System.Diagnostics.PerformanceCounter.Tests
=== TEST EXECUTION SUMMARY ===
   System.Diagnostics.PerformanceCounter.Tests  Total: 90, Errors: 0, Failed: 1, Skipped: 1, Time: 30,024s
Author: ViktorHofer
Assignees: -
Labels:

untriaged, area-System.Diagnostics.PerformanceCounter

Milestone: -

@ViktorHofer ViktorHofer added the blocking-clean-ci Blocking PR or rolling runs of 'runtime' or 'runtime-extra-platforms' label Apr 20, 2022
@danmoseley
Copy link
Member

we already have retries here, but evidently those were successful or not needed as there would have been an exception. So pcc.ReadCategory()) gave us 0 values.

        [Fact]
        public static void InstanceDataCollectionCollection_CopyTo()
        {
            InstanceDataCollectionCollection idcc = GetInstanceDataCollectionCollection();

            InstanceDataCollection[] idc = new InstanceDataCollection[idcc.Values.Count];
            idcc.CopyTo(idc, 0);
            Assert.True(idc.Length > 0);
        }

        public static InstanceDataCollectionCollection GetInstanceDataCollectionCollection()
        {
            PerformanceCounterCategory pcc =  Helpers.RetryOnAllPlatforms(() => new PerformanceCounterCategory("Processor"));
            return Helpers.RetryOnAllPlatforms(() => pcc.ReadCategory());
        }

we should update to retry if there were 0 values. basically add an optional retryWhen delegate to RetryOnAllPlatforms that just passes through to the same named parameter on RetryHelper.Execute. (Tweak if there are other tests that do expect zero values)

@danmoseley
Copy link
Member

#60933 is similar: retries needed on the ctor in those cases.

@carlossanlop carlossanlop added this to the 7.0.0 milestone Jun 1, 2022
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Jun 1, 2022
@smasher164
Copy link
Member

smasher164 commented Jun 1, 2022

we should update to retry if there were 0 values. basically add an optional retryWhen delegate to RetryOnAllPlatforms that just passes through to the same named parameter on RetryHelper.Execute. (Tweak if there are other tests that do expect zero values)

@danmoseley Since retryWhen is only invoked when an exception is thrown, it sounds like the solution should be to

return Helpers.RetryOnAllPlatforms(() =>
{
    var idcc = pcc.ReadCategory();
    if (idcc.Values.Count == 0)
    {
        throw new ZeroDataException;
    }
    return idcc;
});

In that case, passing a retryWhen delegate through wouldn't be needed, since the exception gets caught by the default retryWhen delegate.

Edit: I put up a PR with this approach. Please let me know if there's I'm missing here.

smasher164 added a commit to smasher164/runtime that referenced this issue Jun 1, 2022
Previously, when pcc.ReadCategory() returned 0 values, we did not retry,
which broke the test that asserted its length > 0. This time, we retry
when there are 0 values, by triggering the RetryHelper's exception
handler.

Fixes dotnet#68291
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jun 1, 2022
@danmoseley
Copy link
Member

@smasher164 sounds reasonable.

danmoseley pushed a commit that referenced this issue Jun 2, 2022
…70119)

* fix InstanceDataCollectionCollection_CopyTo by retrying on 0 values

Previously, when pcc.ReadCategory() returned 0 values, we did not retry,
which broke the test that asserted its length > 0. This time, we retry
when there are 0 values, by triggering the RetryHelper's exception
handler.

Fixes #68291

* add ZeroDataException

* Revert "add ZeroDataException"

This reverts commit 43ba3ad.

* throw xUnitException instead of making a custom one

* no need for if statement
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jun 2, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jul 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Diagnostics.PerformanceCounter blocking-clean-ci Blocking PR or rolling runs of 'runtime' or 'runtime-extra-platforms'
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants