Skip to content

CIApp: Testing frameworks integrations test suite and fixes#1317

Merged
tonyredondo merged 57 commits into
masterfrom
tony/ciapp-test-suite
May 21, 2021
Merged

CIApp: Testing frameworks integrations test suite and fixes#1317
tonyredondo merged 57 commits into
masterfrom
tony/ciapp-test-suite

Conversation

@tonyredondo

@tonyredondo tonyredondo commented Mar 25, 2021

Copy link
Copy Markdown
Member

This PR includes:

  • Fixes on scenarios where we were loosing spans.
  • Adds integration tests for each supported testing framework and version.
  • Refactor due the fixes to share similar code between testing framework integrations.

@DataDog/apm-dotnet

@tonyredondo tonyredondo added area:tests unit tests, integration tests area:ci-visibility labels Mar 25, 2021
@tonyredondo tonyredondo self-assigned this Mar 25, 2021
@tonyredondo
tonyredondo force-pushed the tony/ciapp-test-suite branch from fdbb0a8 to 8ff0c56 Compare April 5, 2021 16:57
@tonyredondo tonyredondo added the status:work-in-progress Actively worked on. If this is a PR, no review needed yet. WIP. label Apr 6, 2021
@tonyredondo
tonyredondo force-pushed the tony/ciapp-test-suite branch from b856ceb to 2950176 Compare April 9, 2021 08:41
@tonyredondo
tonyredondo force-pushed the tony/ciapp-test-suite branch from d1ab1ad to f14881f Compare April 20, 2021 13:25
@tonyredondo
tonyredondo force-pushed the tony/ciapp-test-suite branch 4 times, most recently from 7efd8ee to 330d979 Compare May 7, 2021 09:46
@tonyredondo tonyredondo changed the title CIApp: Testing frameworks integrations test suite CIApp: Testing frameworks integrations test suite and fixes May 7, 2021
@tonyredondo tonyredondo removed the status:work-in-progress Actively worked on. If this is a PR, no review needed yet. WIP. label May 7, 2021
@tonyredondo
tonyredondo marked this pull request as ready for review May 7, 2021 13:22
@tonyredondo
tonyredondo requested a review from a team as a code owner May 7, 2021 13:22
@tonyredondo
tonyredondo force-pushed the tony/ciapp-test-suite branch 2 times, most recently from 58e009a to 3f1c0b3 Compare May 11, 2021 16:39
@tonyredondo
tonyredondo force-pushed the tony/ciapp-test-suite branch from 18ba020 to aac2aae Compare May 18, 2021 10:23

@andrewlock andrewlock left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM AFAICT, added a few comments/questions :)

Comment thread Datadog.Trace.proj
Comment thread src/Datadog.Trace.ClrProfiler.Managed/AutoInstrumentation/Testing/Common.cs Outdated
Comment thread test/Datadog.Trace.TestHelpers/EnvironmentHelper.cs Outdated
Comment thread test/test-applications/integrations/Samples.XUnitTests/xunit.runner.json Outdated
@tonyredondo
tonyredondo requested a review from a team May 19, 2021 10:33
span.Type = SpanTypes.Test;
span.SetTraceSamplingPriority(SamplingPriority.AutoKeep);
span.ResourceName = $"{testSuite}.{testName}";
span.SetTag(TestTags.Suite, testSuite);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like an appropriate time to create a new TestTags type that implements ITags, but that doesn't need to happen in this PR

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah all the ITags work came after the test frameworks integrations, I'll update the implementation in another PR.

Comment thread src/Datadog.Trace.ClrProfiler.Managed/Integrations/Testing/NUnitIntegration.cs Outdated
Comment thread src/Datadog.Trace.ClrProfiler.Managed/Integrations/Testing/NUnitIntegration.cs Outdated
Comment thread test/Datadog.Trace.IntegrationTests/Properties/launchSettings.json Outdated

@zacharycmontoya zacharycmontoya left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good! I left some small comments but nothing blocking

@tonyredondo
tonyredondo merged commit acfa227 into master May 21, 2021
@tonyredondo
tonyredondo deleted the tony/ciapp-test-suite branch May 21, 2021 15:40
@andrewlock andrewlock added this to the 1.27.0 milestone Jun 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:ci-visibility area:tests unit tests, integration tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants