Skip to content

Commit

Permalink
Fix tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@alanwest
alanwest committed Oct 25, 2023
1 parent 9b2e211 commit 7d552b3
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 86 deletions.
80 changes: 15 additions & 65 deletions test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@
| :broken_heart: | [16](#16) | Index | /Index | RazorPages | GET /Index | Index |
| :broken_heart: | [17](#17) | PageThatThrowsException | /PageThatThrowsException | RazorPages | GET /PageThatThrowsException | PageThatThrowsException |
| :broken_heart: | [18](#18) | /js/site.js | | RazorPages | GET /js/site.js | /js/site.js |
| :broken_heart: | [19](#19) | /MinimalApi | TBD | MinimalApi | GET /MinimalApi | /MinimalApi |
| :broken_heart: | [20](#20) | /MinimalApi/123 | TBD | MinimalApi | GET /MinimalApi/123 | /MinimalApi/123 |

#### 1

Expand All @@ -32,7 +30,7 @@
"HttpRouteByActionDescriptor": "ConventionalRoute/Default/{id?}",
"DebugInfo": {
"RawText": "{controller=ConventionalRoute}/{action=Default}/{id?}",
"RouteDiagnosticMetadata": "{controller=ConventionalRoute}/{action=Default}/{id?}",
"RouteDiagnosticMetadata": null,
"RouteData": {
"controller": "ConventionalRoute",
"action": "Default"
Expand All @@ -58,7 +56,7 @@
"HttpRouteByActionDescriptor": "ConventionalRoute/ActionWithStringParameter/{id?}",
"DebugInfo": {
"RawText": "{controller=ConventionalRoute}/{action=Default}/{id?}",
"RouteDiagnosticMetadata": "{controller=ConventionalRoute}/{action=Default}/{id?}",
"RouteDiagnosticMetadata": null,
"RouteData": {
"controller": "ConventionalRoute",
"action": "ActionWithStringParameter",
Expand Down Expand Up @@ -88,7 +86,7 @@
"HttpRouteByActionDescriptor": "ConventionalRoute/ActionWithStringParameter/{id?}",
"DebugInfo": {
"RawText": "{controller=ConventionalRoute}/{action=Default}/{id?}",
"RouteDiagnosticMetadata": "{controller=ConventionalRoute}/{action=Default}/{id?}",
"RouteDiagnosticMetadata": null,
"RouteData": {
"controller": "ConventionalRoute",
"action": "ActionWithStringParameter"
Expand Down Expand Up @@ -140,7 +138,7 @@
"HttpRouteByActionDescriptor": "SomePath/{id}/{num:int}",
"DebugInfo": {
"RawText": "SomePath/{id}/{num:int}",
"RouteDiagnosticMetadata": "SomePath/{id}/{num:int}",
"RouteDiagnosticMetadata": null,
"RouteData": {
"controller": "ConventionalRoute",
"action": "ActionWithStringParameter",
Expand Down Expand Up @@ -194,7 +192,7 @@
"HttpRouteByActionDescriptor": "{area:exists}/ControllerForMyArea/Default/{id?}",
"DebugInfo": {
"RawText": "{area:exists}/{controller=ControllerForMyArea}/{action=Default}/{id?}",
"RouteDiagnosticMetadata": "{area:exists}/{controller=ControllerForMyArea}/{action=Default}/{id?}",
"RouteDiagnosticMetadata": null,
"RouteData": {
"controller": "ControllerForMyArea",
"action": "Default",
Expand All @@ -221,7 +219,7 @@
"HttpRouteByActionDescriptor": "{area:exists}/ControllerForMyArea/NonDefault/{id?}",
"DebugInfo": {
"RawText": "{area:exists}/{controller=ControllerForMyArea}/{action=Default}/{id?}",
"RouteDiagnosticMetadata": "{area:exists}/{controller=ControllerForMyArea}/{action=Default}/{id?}",
"RouteDiagnosticMetadata": null,
"RouteData": {
"controller": "ControllerForMyArea",
"area": "MyArea",
Expand All @@ -248,7 +246,7 @@
"HttpRouteByActionDescriptor": "SomePrefix/AnotherArea/Index/{id?}",
"DebugInfo": {
"RawText": "SomePrefix/{controller=AnotherArea}/{action=Index}/{id?}",
"RouteDiagnosticMetadata": "SomePrefix/{controller=AnotherArea}/{action=Index}/{id?}",
"RouteDiagnosticMetadata": null,
"RouteData": {
"area": "AnotherArea",
"controller": "AnotherArea",
Expand All @@ -275,7 +273,7 @@
"HttpRouteByActionDescriptor": "AttributeRoute",
"DebugInfo": {
"RawText": "AttributeRoute",
"RouteDiagnosticMetadata": "AttributeRoute",
"RouteDiagnosticMetadata": null,
"RouteData": {
"action": "Get",
"controller": "AttributeRoute"
Expand All @@ -301,7 +299,7 @@
"HttpRouteByActionDescriptor": "AttributeRoute/Get",
"DebugInfo": {
"RawText": "AttributeRoute/Get",
"RouteDiagnosticMetadata": "AttributeRoute/Get",
"RouteDiagnosticMetadata": null,
"RouteData": {
"action": "Get",
"controller": "AttributeRoute"
Expand All @@ -327,7 +325,7 @@
"HttpRouteByActionDescriptor": "AttributeRoute/Get/{id}",
"DebugInfo": {
"RawText": "AttributeRoute/Get/{id}",
"RouteDiagnosticMetadata": "AttributeRoute/Get/{id}",
"RouteDiagnosticMetadata": null,
"RouteData": {
"action": "Get",
"controller": "AttributeRoute",
Expand Down Expand Up @@ -356,7 +354,7 @@
"HttpRouteByActionDescriptor": "AttributeRoute/{id}/GetWithActionNameInDifferentSpotInTemplate",
"DebugInfo": {
"RawText": "AttributeRoute/{id}/GetWithActionNameInDifferentSpotInTemplate",
"RouteDiagnosticMetadata": "AttributeRoute/{id}/GetWithActionNameInDifferentSpotInTemplate",
"RouteDiagnosticMetadata": null,
"RouteData": {
"action": "GetWithActionNameInDifferentSpotInTemplate",
"controller": "AttributeRoute",
Expand Down Expand Up @@ -385,7 +383,7 @@
"HttpRouteByActionDescriptor": "AttributeRoute/{id}/GetWithActionNameInDifferentSpotInTemplate",
"DebugInfo": {
"RawText": "AttributeRoute/{id}/GetWithActionNameInDifferentSpotInTemplate",
"RouteDiagnosticMetadata": "AttributeRoute/{id}/GetWithActionNameInDifferentSpotInTemplate",
"RouteDiagnosticMetadata": null,
"RouteData": {
"action": "GetWithActionNameInDifferentSpotInTemplate",
"controller": "AttributeRoute",
Expand Down Expand Up @@ -414,7 +412,7 @@
"HttpRouteByActionDescriptor": "/Index",
"DebugInfo": {
"RawText": "",
"RouteDiagnosticMetadata": "",
"RouteDiagnosticMetadata": null,
"RouteData": {
"page": "/Index"
},
Expand All @@ -439,7 +437,7 @@
"HttpRouteByActionDescriptor": "/Index",
"DebugInfo": {
"RawText": "Index",
"RouteDiagnosticMetadata": "Index",
"RouteDiagnosticMetadata": null,
"RouteData": {
"page": "/Index"
},
Expand All @@ -464,7 +462,7 @@
"HttpRouteByActionDescriptor": "/PageThatThrowsException",
"DebugInfo": {
"RawText": "PageThatThrowsException",
"RouteDiagnosticMetadata": "PageThatThrowsException",
"RouteDiagnosticMetadata": null,
"RouteData": {
"page": "/PageThatThrowsException"
},
Expand Down Expand Up @@ -500,51 +498,3 @@
}
}
```

#### 19

```json
{
"HttpMethod": "GET",
"Path": "/MinimalApi",
"HttpRouteByRawText": "/MinimalApi/",
"HttpRouteByControllerActionAndParameters": "",
"HttpRouteByActionDescriptor": "",
"DebugInfo": {
"RawText": "/MinimalApi/",
"RouteDiagnosticMetadata": "/MinimalApi/",
"RouteData": {},
"AttributeRouteInfo": null,
"ActionParameters": null,
"PageActionDescriptorRelativePath": null,
"PageActionDescriptorViewEnginePath": null,
"ControllerActionDescriptorControllerName": null,
"ControllerActionDescriptorActionName": null
}
}
```

#### 20

```json
{
"HttpMethod": "GET",
"Path": "/MinimalApi/123",
"HttpRouteByRawText": "/MinimalApi/{id}",
"HttpRouteByControllerActionAndParameters": "",
"HttpRouteByActionDescriptor": "",
"DebugInfo": {
"RawText": "/MinimalApi/{id}",
"RouteDiagnosticMetadata": "/MinimalApi/{id}",
"RouteData": {
"id": "123"
},
"AttributeRouteInfo": null,
"ActionParameters": null,
"PageActionDescriptorRelativePath": null,
"PageActionDescriptorViewEnginePath": null,
"ControllerActionDescriptorControllerName": null,
"ControllerActionDescriptorActionName": null
}
}
```
7 changes: 7 additions & 0 deletions test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/RouteTestData.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ private static IEnumerable<object[]> GetArgumentsFromTestCaseObject(IEnumerable<
{
foreach (var testCase in input)
{
if (testCase.MinimumDotnetVersion.HasValue && Environment.Version.Major < testCase.MinimumDotnetVersion.Value)
{
continue;
}

result.Add(new object[]
{
testCase,
Expand All @@ -67,6 +72,8 @@ private static IEnumerable<object[]> GetArgumentsFromTestCaseObject(IEnumerable<

public class RouteTestCase
{
public int? MinimumDotnetVersion { get; set; }

public bool Debug { get; set; }

public TestApplicationScenario TestApplicationScenario { get; set; }
Expand Down
6 changes: 5 additions & 1 deletion test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/RoutingTestFixture.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,11 @@ public RoutingTestFixture()
{
foreach (var scenario in Enum.GetValues<TestApplicationScenario>())
{
this.apps.Add(scenario, TestApplicationFactory.CreateApplication(scenario));
var app = TestApplicationFactory.CreateApplication(scenario);
if (app != null)
{
this.apps.Add(scenario, app);
}
}

foreach (var app in this.apps)
Expand Down
34 changes: 20 additions & 14 deletions test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/RoutingTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,10 @@ namespace RouteTests;

public class RoutingTests : IClassFixture<RoutingTestFixture>, IDisposable
{
// TODO: This test currently uses the old conventions. Update it to use the new conventions.
private const string HttpStatusCode = "http.status_code";
private const string HttpMethod = "http.method";
private const string OldHttpStatusCode = "http.status_code";
private const string OldHttpMethod = "http.method";
private const string HttpStatusCode = "http.response.status_code";
private const string HttpMethod = "http.request.method";
private const string HttpRoute = "http.route";

private readonly RoutingTestFixture fixture;
Expand Down Expand Up @@ -56,7 +57,7 @@ public RoutingTests(RoutingTestFixture fixture)

[Theory]
[MemberData(nameof(TestData))]
public async Task TestRoutes(RouteTestData.RouteTestCase testCase, bool skipAsserts = true)
public async Task TestRoutes(RouteTestData.RouteTestCase testCase, bool skipAsserts = true, bool useLegacyConventions = true)
{
await this.fixture.MakeRequest(testCase.TestApplicationScenario, testCase.Path);

Expand All @@ -73,10 +74,10 @@ public async Task TestRoutes(RouteTestData.RouteTestCase testCase, bool skipAsse
this.meterProvider.ForceFlush();

Assert.Single(this.exportedActivities);
Assert.Single(this.exportedMetrics);
var durationMetric = this.exportedMetrics.Single(x => x.Name == "http.server.request.duration" || x.Name == "http.server.duration");

var metricPoints = new List<MetricPoint>();
foreach (var mp in this.exportedMetrics[0].GetMetricPoints())
foreach (var mp in durationMetric.GetMetricPoints())
{
metricPoints.Add(mp);
}
Expand All @@ -86,8 +87,8 @@ public async Task TestRoutes(RouteTestData.RouteTestCase testCase, bool skipAsse
var activity = this.exportedActivities[0];
var metricPoint = metricPoints.First();

this.GetTagsFromActivity(activity, out var activityHttpStatusCode, out var activityHttpMethod, out var activityHttpRoute);
this.GetTagsFromMetricPoint(metricPoint, out var metricHttpStatusCode, out var metricHttpMethod, out var metricHttpRoute);
this.GetTagsFromActivity(useLegacyConventions, activity, out var activityHttpStatusCode, out var activityHttpMethod, out var activityHttpRoute);
this.GetTagsFromMetricPoint(useLegacyConventions && Environment.Version.Major < 8, metricPoint, out var metricHttpStatusCode, out var metricHttpMethod, out var metricHttpRoute);

Assert.Equal(testCase.ExpectedStatusCode, activityHttpStatusCode);
Assert.Equal(testCase.ExpectedStatusCode, metricHttpStatusCode);
Expand Down Expand Up @@ -124,26 +125,31 @@ public void Dispose()
this.meterProvider.Dispose();
}

private void GetTagsFromActivity(Activity activity, out int httpStatusCode, out string httpMethod, out string? httpRoute)
private void GetTagsFromActivity(bool useLegacyConventions, Activity activity, out int httpStatusCode, out string httpMethod, out string? httpRoute)
{
httpStatusCode = Convert.ToInt32(activity.GetTagItem(HttpStatusCode));
httpMethod = (activity.GetTagItem(HttpMethod) as string)!;
var expectedStatusCodeKey = useLegacyConventions ? OldHttpStatusCode : HttpStatusCode;
var expectedHttpMethodKey = useLegacyConventions ? OldHttpMethod : HttpMethod;
httpStatusCode = Convert.ToInt32(activity.GetTagItem(expectedStatusCodeKey));
httpMethod = (activity.GetTagItem(expectedHttpMethodKey) as string)!;
httpRoute = activity.GetTagItem(HttpRoute) as string ?? string.Empty;
}

private void GetTagsFromMetricPoint(MetricPoint metricPoint, out int httpStatusCode, out string httpMethod, out string? httpRoute)
private void GetTagsFromMetricPoint(bool useLegacyConventions, MetricPoint metricPoint, out int httpStatusCode, out string httpMethod, out string? httpRoute)
{
var expectedStatusCodeKey = useLegacyConventions ? OldHttpStatusCode : HttpStatusCode;
var expectedHttpMethodKey = useLegacyConventions ? OldHttpMethod : HttpMethod;

httpStatusCode = 0;
httpMethod = string.Empty;
httpRoute = string.Empty;

foreach (var tag in metricPoint.Tags)
{
if (tag.Key.Equals(HttpStatusCode))
if (tag.Key.Equals(expectedStatusCodeKey))
{
httpStatusCode = Convert.ToInt32(tag.Value);
}
else if (tag.Key.Equals(HttpMethod))
else if (tag.Key.Equals(expectedHttpMethodKey))
{
httpMethod = (tag.Value as string)!;
}
Expand Down
12 changes: 8 additions & 4 deletions test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/TestApplicationFactory.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
// limitations under the License.
// </copyright>

#nullable disable
#nullable enable

using System.Diagnostics;
using Microsoft.AspNetCore.Builder;
Expand Down Expand Up @@ -56,7 +56,7 @@ internal class TestApplicationFactory
? AspNetCoreTestsPath
: Path.Combine(AspNetCoreTestsPath, "RouteTests");

public static WebApplication CreateApplication(TestApplicationScenario config)
public static WebApplication? CreateApplication(TestApplicationScenario config)
{
Debug.Assert(Directory.Exists(ContentRootPath), $"Cannot find ContentRootPath: {ContentRootPath}");
switch (config)
Expand All @@ -66,7 +66,11 @@ public static WebApplication CreateApplication(TestApplicationScenario config)
case TestApplicationScenario.AttributeRouting:
return CreateAttributeRoutingApplication();
case TestApplicationScenario.MinimalApi:
#if NET7_0_OR_GREATER
return CreateMinimalApiApplication();
#else
return null;
#endif
case TestApplicationScenario.RazorPages:
return CreateRazorPagesApplication();
default:
Expand Down Expand Up @@ -125,6 +129,7 @@ private static WebApplication CreateAttributeRoutingApplication()
return app;
}

#if NET7_0_OR_GREATER
private static WebApplication CreateMinimalApiApplication()
{
var builder = WebApplication.CreateBuilder(); // WebApplication.CreateSlimBuilder();
Expand All @@ -134,14 +139,13 @@ private static WebApplication CreateMinimalApiApplication()
app.Urls.Add("http://[::1]:0");
app.UseMiddleware<RouteInfoMiddleware>();

#if NET7_0_OR_GREATER
var api = app.MapGroup("/MinimalApi");
api.MapGet("/", () => Results.Ok());
api.MapGet("/{id}", (int id) => Results.Ok());
#endif

return app;
}
#endif

private static WebApplication CreateRazorPagesApplication()
{
Expand Down
6 changes: 4 additions & 2 deletions test/OpenTelemetry.Instrumentation.AspNetCore.Tests/RouteTests/testcases.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,13 +130,15 @@
"httpMethod": "GET",
"path": "/MinimalApi",
"expectedStatusCode": 200,
"expectedHttpRoute": "TBD"
"expectedHttpRoute": "TBD",
"minimumDotnetVersion": 7
},
{
"testApplicationScenario": "MinimalApi",
"httpMethod": "GET",
"path": "/MinimalApi/123",
"expectedStatusCode": 200,
"expectedHttpRoute": "TBD"
"expectedHttpRoute": "TBD",
"minimumDotnetVersion": 7
}
]

0 comments on commit 7d552b3

Please sign in to comment.