Skip to content

[CI Visibility] Fix MSTest integrarion bug#6336

Merged
tonyredondo merged 2 commits into
masterfrom
tony/civisibility-fix-mstestintegration-bug
Nov 22, 2024
Merged

[CI Visibility] Fix MSTest integrarion bug#6336
tonyredondo merged 2 commits into
masterfrom
tony/civisibility-fix-mstestintegration-bug

Conversation

@tonyredondo

Copy link
Copy Markdown
Member

Summary of changes

This PR fixes a bug found in the MsTest integration where the assemblyName is empty or an invalid filepath.

Reason for change

We found an issue in error tracking about receiving an invalid assemblyName that couldn't be resolved because the file didn't exist.

Implementation details

Now the code validates if the file exist to load it, if not it will try to parse the assembly name, catching all possible exceptions.

Test coverage

This is a hard to test scenario.

@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.

Thanks!

@datadog-ddstaging

datadog-ddstaging Bot commented Nov 22, 2024

Copy link
Copy Markdown

Datadog Report

Branch report: tony/civisibility-fix-mstestintegration-bug
Commit report: a0f8b25
Test service: dd-trace-dotnet

✅ 0 Failed, 449428 Passed, 2734 Skipped, 19h 10m 28.51s Total Time

@andrewlock

Copy link
Copy Markdown
Member

Execution-Time Benchmarks Report ⏱️

Execution-time results for samples comparing the following branches/commits:

Execution-time benchmarks measure the whole time it takes to execute a program. And are intended to measure the one-off costs. Cases where the execution time results for the PR are worse than latest master results are shown in red. The following thresholds were used for comparing the execution times:

  • Welch test with statistical test for significance of 5%
  • Only results indicating a difference greater than 5% and 5 ms are considered.

Note that these results are based on a single point-in-time result for each branch. For full results, see the dashboard.

Graphs show the p99 interval based on the mean and StdDev of the test run, as well as the mean value of the run (shown as a diamond below the graph).

gantt
    title Execution time (ms) FakeDbCommand (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6336) - mean (72ms)  : 61, 84
     .   : milestone, 72,
    master - mean (71ms)  : 64, 78
     .   : milestone, 71,

    section CallTarget+Inlining+NGEN
    This PR (6336) - mean (1,107ms)  : 1087, 1128
     .   : milestone, 1107,
    master - mean (1,109ms)  : 1083, 1135
     .   : milestone, 1109,

Loading
gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6336) - mean (108ms)  : 106, 110
     .   : milestone, 108,
    master - mean (109ms)  : 106, 112
     .   : milestone, 109,

    section CallTarget+Inlining+NGEN
    This PR (6336) - mean (765ms)  : 752, 778
     .   : milestone, 765,
    master - mean (772ms)  : 752, 791
     .   : milestone, 772,

Loading
gantt
    title Execution time (ms) FakeDbCommand (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6336) - mean (92ms)  : 90, 94
     .   : milestone, 92,
    master - mean (92ms)  : 91, 94
     .   : milestone, 92,

    section CallTarget+Inlining+NGEN
    This PR (6336) - mean (723ms)  : 707, 738
     .   : milestone, 723,
    master - mean (726ms)  : 711, 741
     .   : milestone, 726,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6336) - mean (191ms)  : 187, 196
     .   : milestone, 191,
    master - mean (192ms)  : 186, 198
     .   : milestone, 192,

    section CallTarget+Inlining+NGEN
    This PR (6336) - mean (1,212ms)  : 1184, 1240
     .   : milestone, 1212,
    master - mean (1,214ms)  : 1192, 1237
     .   : milestone, 1214,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6336) - mean (276ms)  : 272, 280
     .   : milestone, 276,
    master - mean (278ms)  : 272, 284
     .   : milestone, 278,

    section CallTarget+Inlining+NGEN
    This PR (6336) - mean (938ms)  : 917, 960
     .   : milestone, 938,
    master - mean (949ms)  : 933, 965
     .   : milestone, 949,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6336) - mean (267ms)  : 262, 271
     .   : milestone, 267,
    master - mean (266ms)  : 262, 270
     .   : milestone, 266,

    section CallTarget+Inlining+NGEN
    This PR (6336) - mean (933ms)  : 910, 955
     .   : milestone, 933,
    master - mean (931ms)  : 914, 948
     .   : milestone, 931,

Loading

@tonyredondo
tonyredondo merged commit 765d501 into master Nov 22, 2024
@tonyredondo
tonyredondo deleted the tony/civisibility-fix-mstestintegration-bug branch November 22, 2024 15:14
@github-actions github-actions Bot added this to the vNext-v3 milestone Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants