Add PerformanceCounters Testing #24812

michellemcdaniel · 2017-10-23T16:07:29Z

No description provided.

danmoseley · 2017-10-24T19:52:10Z

src/System.Diagnostics.PerformanceCounter/tests/CounterCreationDataCollectionTests.cs

+        {
+            CounterCreationData[] ccds = { new CounterCreationData("Simple1", "Simple Help", PerformanceCounterType.RawBase), new CounterCreationData("Simple2", "Simple Help", PerformanceCounterType.RawBase) };
+            CounterCreationDataCollection ccdc = new CounterCreationDataCollection(ccds);
+


nit, newlines .. you can search and replace \s*\n\s*\n with \n

danmoseley · 2017-10-24T19:52:25Z

src/System.Diagnostics.PerformanceCounter/tests/CounterCreationDataCollectionTests.cs

+        [ConditionalFact(typeof(AdminHelpers), nameof(AdminHelpers.IsProcessElevated))]
+        public static void PerformanceCounterCategory_CreateCategory()
+        {
+            if ( !PerformanceCounterCategory.Exists("AverageCounter64SampleCategory") ) 


nit, spaces inside parens

danmoseley · 2017-10-24T19:53:54Z

src/System.Diagnostics.PerformanceCounter/tests/InstanceDataTests.cs

+        public static InstanceDataCollection GetInstanceDataCollection()
+        {
+            InstanceDataCollectionCollection idcc = GetInstanceDataCollectionCollection();
+            return idcc["% User Time"];


You might want to try changing your Windows locale to non English (then log in/out) and verify your tests still pass, that will verify that strings like this are not localized.

Our tests must pass on the machines of contributors who aren't using English.

danmoseley · 2017-10-24T19:55:25Z

src/System.Diagnostics.PerformanceCounter/tests/PerformanceCounterCategoryTests.cs

@@ -0,0 +1,368 @@
+// Licensed to the .NET Foundation under one or more agreements.


BTW I noticed lingering use of CAS (code access security) in performancecountercategory.cs:

PerformanceCounterPermission permission = new PerformanceCounterPermission(PerformanceCounterPermissionAccess.Read, machineName, categoryName); permission.Demand();

any stuff that looks like this can be removed.

You can also remove any of these
RuntimeHelpers.PrepareConstrainedRegions();

Can I also remove RegistryPermission, EnvironmentPermission and FileIOPermission, etc?

yes, anything in S.S.Permissions -- remove all the using System.Security.Permissions; and you will find them

danmoseley · 2017-10-24T19:55:46Z

src/System.Diagnostics.PerformanceCounter/tests/PerformanceCounterCategoryTests.cs

+            Assert.Equal("Processor", pcc.CategoryName);
+        }
+
+        [ConditionalFact(typeof(AdminHelpers), nameof(AdminHelpers.IsProcessElevated))]


It's not a big deal, but many of these tests don't need this attribute.

BTW you could replace it with
[ConditionalFact(typeof(Helpers), nameof(Helpers.CanWriteToPerfCounters))]
or similar, akin to what event logs did with [ConditionalFact(typeof(Helpers), nameof(Helpers.SupportsEventLogs))]

Nice thing about this is it makes a single place you can tune when the tests run later.

And it appears you need to -- your tests are failing on Windows Nano. So once you centralize as I usggest, you'll need to add && !PlatformDetection.IsWindowsNanoServer to the check.

danmoseley · 2017-10-24T19:56:18Z

src/System.Diagnostics.PerformanceCounter/tests/PerformanceCounterCategoryTests.cs

+            Assert.Throws<ArgumentNullException>(() => PerformanceCounterCategory.Create("category name", "Category help", PerformanceCounterCategoryType.SingleInstance, null));
+            Assert.Throws<InvalidOperationException>(() => PerformanceCounterCategory.Create("Processor", "Category help", PerformanceCounterCategoryType.MultiInstance, "Interrupts/sec", "counter help"));
+
+            string maxCounter = "";


replace this with just new string('a', 32768)

danmoseley · 2017-10-24T19:58:31Z

src/System.Diagnostics.PerformanceCounter/tests/PerformanceCounterCategoryTests.cs

+            var name = nameof(PerformanceCounterCategory_GetCounterHelp) + "_Counter";
+            var category = name + "_Category";
+
+            if (PerformanceCounterCategory.Exists(category))


This category exists/delete pattern you are using all over, you could make it into a private helper function

Ah you have one: DeleteCategory()

danmoseley

Looks good! nearly done with perf counters!

danmoseley · 2017-10-24T20:10:31Z

Looks like Windows 7 has some interesting quirks/limitations that your tests will need to work around --see test results.

michellemcdaniel · 2017-10-25T21:25:23Z

@dotnet-bot test Windows x64 Debug Build please

michellemcdaniel · 2017-10-25T21:35:11Z

@danmosemsft The PerfCounter tests tend to be a little flakey, even locally. I don't know if it's because it takes a little while for the system to actually register the PerfCounter, outside of the code, or what else is going on. I changed the if(!PerfCounterCounter.Exists) statement to be a while, and that seems to have addressed some of the issue, because it makes sure that we actually have created the category before trying to create the counter, but I'm not sure if that's the right way to address the issue. It's also not the best track for the PerformanceCounterCategory tests, which also have this issue. I wonder if putting a short sleep in the tests would give the system time, but I don't want to add unnecessary time to the tests. Thoughts?

danmoseley · 2017-10-25T21:50:51Z

It's OK to poll so long as you check first. ie., it's OK to do

success = doBlah()
while (!success && tries < 3)
{
tries++
Thread.Sleep(100)
success = doBlah()
}

and hopefully it rarely actually needs to sleep.

@Anipik found he had to do this for some of his event log tests. MSDN even said there was a 6 second delay in some cases.

danmoseley · 2017-10-25T21:51:06Z

See if that pattern is possible (hopefully in a couple places not sprayed all over)

michellemcdaniel · 2017-10-25T21:54:20Z

Ah ok. I'll try it out. Thanks.

danmoseley · 2017-10-25T22:10:44Z

If the tests are then slow we can just make them outer loop, so they don't bother most people. But they must be 100% reliable.

danmoseley · 2017-10-26T03:16:46Z

@dotnet-bot test OSX x64 Debug Build (access dnied)

michellemcdaniel · 2017-10-26T16:02:05Z

@dotnet-bot test Windows x86 Release Build please

michellemcdaniel · 2017-10-26T18:03:27Z

@dotnet-bot test this please

michellemcdaniel · 2017-10-26T19:59:07Z

@dotnet-bot test Windows x64 Debug Build please

danmoseley · 2017-10-27T23:54:42Z

src/System.Diagnostics.PerformanceCounter/tests/CounterSampleCalculatorTests.cs

@@ -99,15 +99,17 @@ public static PerformanceCounter CreateCounter(string name, PerformanceCounterTy
            }

            DeleteCategory(name);
+            PerformanceCounterCategory.Create(category, "description", PerformanceCounterCategoryType.SingleInstance, ccdc);


I realize you're experimenting at this point but I hope when you're done any sleep/retries are abstracted from the tests either by having methods like "CreateCategory" that they call that guarantee it's visible when they complete, or worst case by having a "PollUntilTrue" method they call that takes a delegate to check for the case they want and it will poll or timeout then return. Timeout should cause assert failure.

michellemcdaniel · 2017-10-31T15:21:38Z

@dotnet-bot test Windows x64 Debug Build please
@dotnet-bot test Windows x86 Release Build please

danmoseley · 2017-10-31T16:40:36Z

LGTM Please remove Console.WriteLines before committing...

michellemcdaniel · 2017-11-01T21:02:23Z

@dotnet-bot test Windows x86 Release Build please

danmoseley · 2017-11-02T20:36:38Z

@adiaaida I think Helpers.cs is missing using Xunit; ..

danmoseley · 2017-11-02T21:35:30Z

@dotnet/dnceng have you seen this?

  Restoring packages for /Users/dotnet-bot/j/workspace/dotnet_corefx/master/osx-TGroup_netcoreapp+CGroup_Debug+AGroup_x64+TestOuter_false_prtest/init-tools.msbuild...
  Retrying 'FindPackagesByIdAsync' for source 'https://dotnetmyget.blob.core.windows.net/artifacts/dotnet-buildtools/nuget/v3/flatcontainer/microsoft.dotnet.build.tasks.feed/index.json'.
  Access to the path '/tmp/NuGet/TempCache/2f3f9dbe-352f-4004-ae7d-1c899166cb67' is denied.
    Permission denied
  Retrying 'FindPackagesByIdAsync' for source 'https://dotnetmyget.blob.core.windows.net/artifacts/dotnet-buildtools/nuget/v3/flatcontainer/microsoft.dotnet.buildtools/index.json'.
  Access to the path '/tmp/NuGet/TempCache/2f3f9dbe-352f-4004-ae7d-1c899166cb67' is denied.
    Permission denied
  Retrying 'FindPackagesByIdAsync' for source 'https://dotnetmyget.blob.core.windows.net/artifacts/dotnet-buildtools/nuget/v3/flatcontainer/microsoft.dotnet.build.tasks.feed/index.json'.
  Access to the path '/tmp/NuGet/TempCache/2f3f9dbe-352f-4004-ae7d-1c899166cb67' is denied.
    Permission denied
  Retrying 'FindPackagesByIdAsync' for source 'https://dotnetmyget.blob.core.windows.net/artifacts/dotnet-buildtools/nuget/v3/flatcontainer/microsoft.dotnet.buildtools/index.json'.
  Access to the path '/tmp/NuGet/TempCache/2f3f9dbe-352f-4004-ae7d-1c899166cb67' is denied.
    Permission denied

https://ci3.dot.net/job/dotnet_corefx/job/master/job/osx-TGroup_netcoreapp+CGroup_Debug+AGroup_x64+TestOuter_false_prtest/5011/consoleText

michellemcdaniel · 2017-11-03T16:56:25Z

@dotnet-bot test Windows x64 Debug Build please
@dotnet-bot test Windows x86 Release Build please

michellemcdaniel · 2017-11-06T17:16:25Z

@dotnet-bot test Windows x64 Debug Build please
@dotnet-bot test Windows x86 Release Build please

michellemcdaniel · 2017-11-07T17:23:44Z

@dotnet-bot test Windows x64 Debug Build please
@dotnet-bot test Windows x86 Release Build please

danmoseley · 2017-11-07T17:34:47Z

I opened a core-eng issue for Caused by: com.fasterxml.jackson.databind.JsonMappingException: Numeric value (4302832145) out of range of int

michellemcdaniel · 2017-11-07T17:42:07Z

@dotnet-bot test Windows x64 Debug Build please
@dotnet-bot test Windows x86 Release Build please

michellemcdaniel · 2017-11-07T20:25:40Z

@dotnet-bot test Windows x64 Debug Build please
@dotnet-bot test Windows x86 Release Build please

michellemcdaniel · 2017-11-08T00:58:35Z

@dotnet-bot test Windows x64 Debug Build please
@dotnet-bot test Windows x86 Release Build please

michellemcdaniel · 2017-11-08T17:04:03Z

@dotnet-bot test Windows x64 Debug Build please
@dotnet-bot test Windows x86 Release Build please

michellemcdaniel · 2017-11-08T19:28:45Z

@danmosemsft I've gotten the tests to pass in the lab 3 times now, so I am going to check this in now.

danmoseley · 2017-11-08T19:52:19Z

Sounds good!

Add PerformanceCounters Testing Commit migrated from dotnet/corefx@5617d39

dnfclas added the cla-already-signed label Oct 23, 2017

michellemcdaniel requested a review from danmoseley October 23, 2017 16:07

danmoseley reviewed Oct 24, 2017

View reviewed changes

danmoseley approved these changes Oct 24, 2017

View reviewed changes

michellemcdaniel force-pushed the addPerformanceCounterTests branch from 8e7a4b0 to f640d6d Compare October 25, 2017 16:36

michellemcdaniel force-pushed the addPerformanceCounterTests branch from f640d6d to 7d9cf82 Compare October 25, 2017 22:39

danmoseley reviewed Oct 27, 2017

View reviewed changes

karelz added the area-System.Diagnostics.Tracing label Oct 28, 2017

karelz assigned michellemcdaniel Oct 28, 2017

karelz added this to the 2.1.0 milestone Oct 28, 2017

michellemcdaniel force-pushed the addPerformanceCounterTests branch from 7e7f26e to ffa2931 Compare October 31, 2017 20:41

michellemcdaniel force-pushed the addPerformanceCounterTests branch from ffa2931 to caa72fb Compare November 1, 2017 14:30

michellemcdaniel force-pushed the addPerformanceCounterTests branch from 5b401fb to caa72fb Compare November 2, 2017 20:35

michellemcdaniel added 7 commits November 6, 2017 14:26

Add PerformanceCounters Testing

9ff7734

fix space

b63df37

Move assert

89afdc3

fix

ce92fbc

Change test

227e1c1

remove test

c4be561

add retry delegate

80cd489

michellemcdaniel force-pushed the addPerformanceCounterTests branch from a98d9ef to 80cd489 Compare November 6, 2017 23:06

michellemcdaniel added 2 commits November 6, 2017 16:01

adjust tests

723b056

add more retry calls

a984536

add more retry calls

cb1e412

michellemcdaniel added 2 commits November 7, 2017 13:18

Additional fixes

8a10511

add more retry calls

661a8ed

michellemcdaniel merged commit 5617d39 into dotnet:master Nov 8, 2017

picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022

Add PerformanceCounters Testing (dotnet/corefx#24812)

8797194

Add PerformanceCounters Testing Commit migrated from dotnet/corefx@5617d39

		@@ -0,0 +1,368 @@
		// Licensed to the .NET Foundation under one or more agreements.

Add PerformanceCounters Testing #24812

Add PerformanceCounters Testing #24812

Conversation

michellemcdaniel commented Oct 23, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danmoseley left a comment

Choose a reason for hiding this comment

danmoseley commented Oct 24, 2017

michellemcdaniel commented Oct 25, 2017

michellemcdaniel commented Oct 25, 2017

danmoseley commented Oct 25, 2017 • edited Loading

danmoseley commented Oct 25, 2017

michellemcdaniel commented Oct 25, 2017

danmoseley commented Oct 25, 2017

danmoseley commented Oct 26, 2017

michellemcdaniel commented Oct 26, 2017

michellemcdaniel commented Oct 26, 2017

michellemcdaniel commented Oct 26, 2017

Choose a reason for hiding this comment

michellemcdaniel commented Oct 31, 2017

danmoseley commented Oct 31, 2017

michellemcdaniel commented Nov 1, 2017

danmoseley commented Nov 2, 2017

danmoseley commented Nov 2, 2017

michellemcdaniel commented Nov 3, 2017

michellemcdaniel commented Nov 6, 2017

michellemcdaniel commented Nov 7, 2017

danmoseley commented Nov 7, 2017

michellemcdaniel commented Nov 7, 2017

michellemcdaniel commented Nov 7, 2017

michellemcdaniel commented Nov 8, 2017

michellemcdaniel commented Nov 8, 2017

michellemcdaniel commented Nov 8, 2017

danmoseley commented Nov 8, 2017

danmoseley commented Oct 25, 2017 •

edited

Loading