From 06adea8d6f8e3f9abeb55ca79dc4345fd6de3948 Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Tue, 2 Jun 2020 18:12:05 -0700 Subject: [PATCH] Enhance DiagnosticSource Test readability (#37294) * Enhance DiagnosticSource Test readability Use the C# 8.0 using statement feature to make the code more readable * Address the feedback * More feedback addressing --- .../tests/ActivitySourceTests.cs | 319 ++++++++---------- 1 file changed, 141 insertions(+), 178 deletions(-) diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivitySourceTests.cs b/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivitySourceTests.cs index daaaa0dda5b14..030e20d77a161 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivitySourceTests.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivitySourceTests.cs @@ -20,18 +20,15 @@ public class ActivitySourceTests : IDisposable public void TestConstruction() { RemoteExecutor.Invoke(() => { - using (ActivitySource as1 = new ActivitySource("Source1")) - { - Assert.Equal("Source1", as1.Name); - Assert.Equal(String.Empty, as1.Version); - Assert.False(as1.HasListeners()); - using (ActivitySource as2 = new ActivitySource("Source2", "1.1.1.2")) - { - Assert.Equal("Source2", as2.Name); - Assert.Equal("1.1.1.2", as2.Version); - Assert.False(as2.HasListeners()); - } - } + using ActivitySource as1 = new ActivitySource("Source1"); + Assert.Equal("Source1", as1.Name); + Assert.Equal(String.Empty, as1.Version); + Assert.False(as1.HasListeners()); + + using ActivitySource as2 = new ActivitySource("Source2", "1.1.1.2"); + Assert.Equal("Source2", as2.Name); + Assert.Equal("1.1.1.2", as2.Version); + Assert.False(as2.HasListeners()); }).Dispose(); } @@ -39,20 +36,17 @@ public void TestConstruction() public void TestStartActivityWithNoListener() { RemoteExecutor.Invoke(() => { - using (ActivitySource aSource = new ActivitySource("SourceActivity")) - { - Assert.Equal("SourceActivity", aSource.Name); - Assert.Equal(string.Empty, aSource.Version); - Assert.False(aSource.HasListeners()); + using ActivitySource aSource = new ActivitySource("SourceActivity"); + Assert.Equal("SourceActivity", aSource.Name); + Assert.Equal(string.Empty, aSource.Version); + Assert.False(aSource.HasListeners()); - Activity current = Activity.Current; - using (Activity a1 = aSource.StartActivity("a1")) - { - // no listeners, we should get null activity. - Assert.Null(a1); - Assert.Equal(Activity.Current, current); - } - } + Activity current = Activity.Current; + + // no listeners, we should get null activity. + using Activity a1 = aSource.StartActivity("a1"); + Assert.Null(a1); + Assert.Equal(Activity.Current, current); }).Dispose(); } @@ -60,27 +54,21 @@ public void TestStartActivityWithNoListener() public void TestActivityWithListenerNoActivityCreate() { RemoteExecutor.Invoke(() => { - using (ActivitySource aSource = new ActivitySource("SourceActivityListener")) - { - Assert.False(aSource.HasListeners()); + using ActivitySource aSource = new ActivitySource("SourceActivityListener"); + Assert.False(aSource.HasListeners()); - using (ActivityListener listener = new ActivityListener - { - ActivityStarted = activity => Assert.NotNull(activity), - ActivityStopped = activity => Assert.NotNull(activity), - ShouldListenTo = (activitySource) => object.ReferenceEquals(aSource, activitySource), - GetRequestedDataUsingParentId = (ref ActivityCreationOptions activityOptions) => ActivityDataRequest.None, - GetRequestedDataUsingContext = (ref ActivityCreationOptions activityOptions) => ActivityDataRequest.None - } - ) - { - ActivitySource.AddActivityListener(listener); - Assert.True(aSource.HasListeners()); + using ActivityListener listener = new ActivityListener(); + listener.ActivityStarted = activity => Assert.NotNull(activity); + listener.ActivityStopped = activity => Assert.NotNull(activity); + listener.ShouldListenTo = (activitySource) => object.ReferenceEquals(aSource, activitySource); + listener.GetRequestedDataUsingParentId = (ref ActivityCreationOptions activityOptions) => ActivityDataRequest.None; + listener.GetRequestedDataUsingContext = (ref ActivityCreationOptions activityOptions) => ActivityDataRequest.None; - // The listener is not allowing to create a new Activity. - Assert.Null(aSource.StartActivity("nullActivity")); - } - } + ActivitySource.AddActivityListener(listener); + Assert.True(aSource.HasListeners()); + + // The listener is not allowing to create a new Activity. + Assert.Null(aSource.StartActivity("nullActivity")); }).Dispose(); } @@ -93,51 +81,47 @@ public void TestActivityWithListenerActivityCreateAndAllDataRequested() int counter = 0; Assert.False(aSource.HasListeners()); - using (ActivityListener listener = new ActivityListener - { - ActivityStarted = activity => counter++, - ActivityStopped = activity => counter--, - ShouldListenTo = (activitySource) => object.ReferenceEquals(aSource, activitySource), - GetRequestedDataUsingParentId = (ref ActivityCreationOptions activityOptions) => ActivityDataRequest.AllDataAndRecorded, - GetRequestedDataUsingContext = (ref ActivityCreationOptions activityOptions) => ActivityDataRequest.AllDataAndRecorded - } - ) + using ActivityListener listener = new ActivityListener(); + listener.ActivityStarted = activity => counter++; + listener.ActivityStopped = activity => counter--; + listener.ShouldListenTo = (activitySource) => object.ReferenceEquals(aSource, activitySource); + listener.GetRequestedDataUsingParentId = (ref ActivityCreationOptions activityOptions) => ActivityDataRequest.AllDataAndRecorded; + listener.GetRequestedDataUsingContext = (ref ActivityCreationOptions activityOptions) => ActivityDataRequest.AllDataAndRecorded; + + ActivitySource.AddActivityListener(listener); + + Assert.True(aSource.HasListeners()); + + using (Activity activity = aSource.StartActivity("AllDataRequestedActivity")) { - ActivitySource.AddActivityListener(listener); + Assert.NotNull(activity); + Assert.True(activity.IsAllDataRequested); + Assert.Equal(1, counter); + + Assert.Equal(0, activity.Tags.Count()); + Assert.Equal(0, activity.Baggage.Count()); + + Assert.True(object.ReferenceEquals(activity, activity.AddTag("key", "value"))); + Assert.True(object.ReferenceEquals(activity, activity.AddBaggage("key", "value"))); - Assert.True(aSource.HasListeners()); + Assert.Equal(1, activity.Tags.Count()); + Assert.Equal(1, activity.Baggage.Count()); - using (Activity activity = aSource.StartActivity("AllDataRequestedActivity")) + using (Activity activity1 = aSource.StartActivity("AllDataRequestedActivity1")) { - Assert.NotNull(activity); - Assert.True(activity.IsAllDataRequested); - Assert.Equal(1, counter); - - Assert.Equal(0, activity.Tags.Count()); - Assert.Equal(0, activity.Baggage.Count()); - - Assert.True(object.ReferenceEquals(activity, activity.AddTag("key", "value"))); - Assert.True(object.ReferenceEquals(activity, activity.AddBaggage("key", "value"))); - - Assert.Equal(1, activity.Tags.Count()); - Assert.Equal(1, activity.Baggage.Count()); - - using (Activity activity1 = aSource.StartActivity("AllDataRequestedActivity1")) - { - Assert.NotNull(activity1); - Assert.True(activity1.IsAllDataRequested); - Assert.Equal(2, counter); - - Assert.Equal(0, activity1.Links.Count()); - Assert.Equal(0, activity1.Events.Count()); - Assert.True(object.ReferenceEquals(activity1, activity1.AddEvent(new ActivityEvent("e1")))); - Assert.Equal(1, activity1.Events.Count()); - } - Assert.Equal(1, counter); + Assert.NotNull(activity1); + Assert.True(activity1.IsAllDataRequested); + Assert.Equal(2, counter); + + Assert.Equal(0, activity1.Links.Count()); + Assert.Equal(0, activity1.Events.Count()); + Assert.True(object.ReferenceEquals(activity1, activity1.AddEvent(new ActivityEvent("e1")))); + Assert.Equal(1, activity1.Events.Count()); } - - Assert.Equal(0, counter); + Assert.Equal(1, counter); } + + Assert.Equal(0, counter); } }).Dispose(); } @@ -151,27 +135,20 @@ public void TestActivitySourceAttachedObject() Assert.Equal("", new Activity("a3").Source.Name); Assert.Equal(string.Empty, new Activity("a4").Source.Version); - using (ActivitySource aSource = new ActivitySource("SourceToTest", "1.2.3.4")) - { - //Ensure at least we have a listener to allow Activity creation - using (ActivityListener listener = new ActivityListener - { - ActivityStarted = activity => Assert.NotNull(activity), - ActivityStopped = activity => Assert.NotNull(activity), - ShouldListenTo = (activitySource) => object.ReferenceEquals(aSource, activitySource), - GetRequestedDataUsingParentId = (ref ActivityCreationOptions activityOptions) => ActivityDataRequest.AllData, - GetRequestedDataUsingContext = (ref ActivityCreationOptions activityOptions) => ActivityDataRequest.AllData - } - ) - { - ActivitySource.AddActivityListener(listener); + using ActivitySource aSource = new ActivitySource("SourceToTest", "1.2.3.4"); - using (Activity activity = aSource.StartActivity("ActivityToTest")) - { - Assert.True(object.ReferenceEquals(aSource, activity.Source)); - } - } - } + // Ensure at least we have a listener to allow Activity creation + using ActivityListener listener = new ActivityListener(); + listener.ActivityStarted = activity => Assert.NotNull(activity); + listener.ActivityStopped = activity => Assert.NotNull(activity); + listener.ShouldListenTo = (activitySource) => object.ReferenceEquals(aSource, activitySource); + listener.GetRequestedDataUsingParentId = (ref ActivityCreationOptions activityOptions) => ActivityDataRequest.AllData; + listener.GetRequestedDataUsingContext = (ref ActivityCreationOptions activityOptions) => ActivityDataRequest.AllData; + + ActivitySource.AddActivityListener(listener); + + using Activity activity = aSource.StartActivity("ActivityToTest"); + Assert.True(object.ReferenceEquals(aSource, activity.Source)); }).Dispose(); } @@ -182,16 +159,14 @@ public void TestListeningToConstructedActivityEvents() int activityStartCount = 0; int activityStopCount = 0; - using (ActivityListener listener = new ActivityListener - { - ActivityStarted = activity => activityStartCount++, - ActivityStopped = activity => activityStopCount++, - ShouldListenTo = (activitySource) => activitySource.Name == "" && activitySource.Version == "", - GetRequestedDataUsingParentId = (ref ActivityCreationOptions activityOptions) => ActivityDataRequest.AllData, - GetRequestedDataUsingContext = (ref ActivityCreationOptions activityOptions) => ActivityDataRequest.AllData - } - ) + using (ActivityListener listener = new ActivityListener()) { + listener.ActivityStarted = activity => activityStartCount++; + listener.ActivityStopped = activity => activityStopCount++; + listener.ShouldListenTo = (activitySource) => activitySource.Name == "" && activitySource.Version == ""; + listener.GetRequestedDataUsingParentId = (ref ActivityCreationOptions activityOptions) => ActivityDataRequest.AllData; + listener.GetRequestedDataUsingContext = (ref ActivityCreationOptions activityOptions) => ActivityDataRequest.AllData; + ActivitySource.AddActivityListener(listener); Assert.Equal(0, activityStartCount); @@ -263,7 +238,7 @@ public void TestExpectedListenersReturnValues() { Assert.False(a2.IsAllDataRequested); Assert.True((a2.ActivityTraceFlags & ActivityTraceFlags.Recorded) == 0); - + Assert.Equal(2, activityStartCount); Assert.Equal(0, activityStopCount); } @@ -285,7 +260,7 @@ public void TestExpectedListenersReturnValues() { Assert.True(a3.IsAllDataRequested); Assert.True((a3.ActivityTraceFlags & ActivityTraceFlags.Recorded) == 0); - + Assert.Equal(5, activityStartCount); Assert.Equal(2, activityStopCount); } @@ -329,58 +304,52 @@ public void TestActivityCreationProperties() RemoteExecutor.Invoke(() => { ActivitySource source = new ActivitySource("MultipleListenerSource"); - using (ActivityListener listener = new ActivityListener - { - ActivityStarted = activity => Assert.NotNull(activity), - ActivityStopped = activity => Assert.NotNull(activity), - ShouldListenTo = (activitySource) => true, - GetRequestedDataUsingParentId = (ref ActivityCreationOptions activityOptions) => ActivityDataRequest.AllData, - GetRequestedDataUsingContext = (ref ActivityCreationOptions activityOptions) => ActivityDataRequest.AllData - } - ) - { - ActivitySource.AddActivityListener(listener); + using ActivityListener listener = new ActivityListener(); + listener.ActivityStarted = activity => Assert.NotNull(activity); + listener.ActivityStopped = activity => Assert.NotNull(activity); + listener.ShouldListenTo = (activitySource) => true; + listener.GetRequestedDataUsingParentId = (ref ActivityCreationOptions activityOptions) => ActivityDataRequest.AllData; + listener.GetRequestedDataUsingContext = (ref ActivityCreationOptions activityOptions) => ActivityDataRequest.AllData; - ActivityContext ctx = new ActivityContext(ActivityTraceId.CreateRandom(), ActivitySpanId.CreateRandom(), ActivityTraceFlags.Recorded, "key0-value0"); + ActivitySource.AddActivityListener(listener); - List links = new List(); - links.Add(new ActivityLink(new ActivityContext(ActivityTraceId.CreateRandom(), ActivitySpanId.CreateRandom(), ActivityTraceFlags.None, "key1-value1"))); - links.Add(new ActivityLink(new ActivityContext(ActivityTraceId.CreateRandom(), ActivitySpanId.CreateRandom(), ActivityTraceFlags.None, "key2-value2"))); + ActivityContext ctx = new ActivityContext(ActivityTraceId.CreateRandom(), ActivitySpanId.CreateRandom(), ActivityTraceFlags.Recorded, "key0-value0"); - List> attributes = new List>(); - attributes.Add(new KeyValuePair("tag1", "tagValue1")); - attributes.Add(new KeyValuePair("tag2", "tagValue2")); - attributes.Add(new KeyValuePair("tag3", "tagValue3")); + List links = new List(); + links.Add(new ActivityLink(new ActivityContext(ActivityTraceId.CreateRandom(), ActivitySpanId.CreateRandom(), ActivityTraceFlags.None, "key1-value1"))); + links.Add(new ActivityLink(new ActivityContext(ActivityTraceId.CreateRandom(), ActivitySpanId.CreateRandom(), ActivityTraceFlags.None, "key2-value2"))); - using (Activity activity = source.StartActivity("a1", ActivityKind.Client, ctx, attributes, links)) - { - Assert.NotNull(activity); - Assert.Equal("a1", activity.OperationName); - Assert.Equal("a1", activity.DisplayName); - Assert.Equal(ActivityKind.Client, activity.Kind); - - Assert.Equal(ctx.TraceId, activity.TraceId); - Assert.Equal(ctx.SpanId, activity.ParentSpanId); - Assert.Equal(ctx.TraceFlags, activity.ActivityTraceFlags); - Assert.Equal(ctx.TraceState, activity.TraceStateString); - Assert.Equal(ActivityIdFormat.W3C, activity.IdFormat); + List> attributes = new List>(); + attributes.Add(new KeyValuePair("tag1", "tagValue1")); + attributes.Add(new KeyValuePair("tag2", "tagValue2")); + attributes.Add(new KeyValuePair("tag3", "tagValue3")); - foreach (KeyValuePair pair in attributes) - { - Assert.NotEqual(default, activity.Tags.FirstOrDefault((p) => pair.Key == p.Key && pair.Value == pair.Value)); - } - - foreach (ActivityLink link in links) - { - Assert.NotEqual(default, activity.Links.FirstOrDefault((l) => link == l)); - } + using (Activity activity = source.StartActivity("a1", ActivityKind.Client, ctx, attributes, links)) + { + Assert.NotNull(activity); + Assert.Equal("a1", activity.OperationName); + Assert.Equal("a1", activity.DisplayName); + Assert.Equal(ActivityKind.Client, activity.Kind); + + Assert.Equal(ctx.TraceId, activity.TraceId); + Assert.Equal(ctx.SpanId, activity.ParentSpanId); + Assert.Equal(ctx.TraceFlags, activity.ActivityTraceFlags); + Assert.Equal(ctx.TraceState, activity.TraceStateString); + Assert.Equal(ActivityIdFormat.W3C, activity.IdFormat); + + foreach (KeyValuePair pair in attributes) + { + Assert.NotEqual(default, activity.Tags.FirstOrDefault((p) => pair.Key == p.Key && pair.Value == pair.Value)); } - using (Activity activity = source.StartActivity("a2", ActivityKind.Client, "NoW3CParentId", attributes, links)) + foreach (ActivityLink link in links) { - Assert.Equal(ActivityIdFormat.Hierarchical, activity.IdFormat); + Assert.NotEqual(default, activity.Links.FirstOrDefault((l) => link == l)); } } + + using Activity activity1 = source.StartActivity("a2", ActivityKind.Client, "NoW3CParentId", attributes, links); + Assert.Equal(ActivityIdFormat.Hierarchical, activity1.IdFormat); }).Dispose(); } @@ -388,32 +357,26 @@ public void TestActivityCreationProperties() public void TestDefaultParentContext() { RemoteExecutor.Invoke(() => { - using (ActivitySource aSource = new ActivitySource("ParentContext")) + using ActivitySource aSource = new ActivitySource("ParentContext"); + using ActivityListener listener = new ActivityListener(); + + listener.ShouldListenTo = (activitySource) => activitySource.Name == "ParentContext"; + listener.GetRequestedDataUsingContext = (ref ActivityCreationOptions activityOptions) => { - using (ActivityListener listener = new ActivityListener + Activity c = Activity.Current; + if (c != null) { - ShouldListenTo = (activitySource) => activitySource.Name == "ParentContext", - GetRequestedDataUsingContext = (ref ActivityCreationOptions activityOptions) => - { - Activity c = Activity.Current; - if (c != null) - { - Assert.Equal(c.Context, activityOptions.Parent); - } + Assert.Equal(c.Context, activityOptions.Parent); + } - return ActivityDataRequest.AllData; - } - }) - { - ActivitySource.AddActivityListener(listener); + return ActivityDataRequest.AllData; + }; - using (Activity a = aSource.StartActivity("a", ActivityKind.Server, new ActivityContext(ActivityTraceId.CreateRandom(), ActivitySpanId.CreateRandom(), 0))) - using (Activity b = aSource.StartActivity("b")) - { - Assert.Equal(a.Context, b.Parent.Context); - } - } - } + ActivitySource.AddActivityListener(listener); + + using Activity a = aSource.StartActivity("a", ActivityKind.Server, new ActivityContext(ActivityTraceId.CreateRandom(), ActivitySpanId.CreateRandom(), 0)); + using Activity b = aSource.StartActivity("b"); + Assert.Equal(a.Context, b.Parent.Context); }).Dispose(); }