Conversation
Evangelink
requested review from
Evangelink,
MarcoRossignoli and
Youssef1313
as code owners
|
TODO: Review number of tests + content of nupkg FYI @Youssef1313 |
| #endif | ||
| builder.AddCrashDumpProvider(); | ||
| builder.AddAppInsightsTelemetryProvider(); | ||
| builder.AddCrashDumpProvider(ignoreIfNotSupported: true); |
There was a problem hiding this comment.
I'm curious why ignoreIfNotSupported was not needed previously
There was a problem hiding this comment.
Probably because crash dump is passive (unlike hang dump), so what happens is that on activation you set env variables and hope that some crash dumps will appear in the dump folder (those will be created by the built-in .net crash reporter). This setup won't fail in native aot, and we probably don't test that it actually works there because we don't expect it to work?
If the process was more active, like in VSTest where we use procdump for .net framework processes, then even the setup would probably fail in native aot, or other incompatible scenario, and we would see it fail early, even without explicitly testing for it.
| builder.AddAppInsightsTelemetryProvider(); | ||
| builder.AddCrashDumpProvider(ignoreIfNotSupported: true); | ||
| builder.AddHangDumpProvider(); | ||
| builder.AddRetryProvider(); |
There was a problem hiding this comment.
@Evangelink Do we want to use re-try for unit tests? I'm afraid this could hide race conditions and flaky tests that indicate real issues
| builder.AddCodeCoverageProvider(); | ||
| #endif | ||
| builder.AddCrashDumpProvider(); | ||
| builder.AddAppInsightsTelemetryProvider(); |
There was a problem hiding this comment.
@Evangelink Any reason telemetry wasn't enabled previously? Is it intentional for our tests to opt-out for telemetry? It's already enabled for two other test projects
| #if ENABLE_CODECOVERAGE | ||
| builder.AddCodeCoverageProvider(); | ||
| #endif | ||
| Console.WriteLine("NATIVE_AOT disabled"); |
| <ProjectReference Include="$(RepoRoot)src\Platform\Microsoft.Testing.Platform\Microsoft.Testing.Platform.csproj" /> | ||
| <ProjectReference Include="$(RepoRoot)src\Platform\Microsoft.Testing.Extensions.TrxReport.Abstractions\Microsoft.Testing.Extensions.TrxReport.Abstractions.csproj" /> | ||
| <ProjectReference Include="$(RepoRoot)src\Platform\Microsoft.Testing.Extensions.Telemetry\Microsoft.Testing.Extensions.Telemetry.csproj" /> | ||
| <ProjectReference Include="$(RepoRoot)src\Platform\Microsoft.Testing.Extensions.Telemetry\Microsoft.Testing.Extensions.Telemetry.csproj" ReferenceOutputAssembly="false" /> |
There was a problem hiding this comment.
Does this affect the publicly visible dependencies of the VSTestBridge NuGet package?
|
|
||
| <ItemGroup> | ||
| <ProjectReference Include="$(RepoRoot)src\Platform\Microsoft.Testing.Platform\Microsoft.Testing.Platform.csproj" /> | ||
| <ProjectReference Include="$(RepoRoot)src\Platform\Microsoft.Testing.Extensions.TrxReport.Abstractions\Microsoft.Testing.Extensions.TrxReport.Abstractions.csproj" /> |
There was a problem hiding this comment.
Same here. Does this affect the publicly visible dependencies of CrashDump?
Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…mblies (microsoft#5150) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…t/arcade (microsoft#5147) Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
…lyzers (microsoft#5153) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…soft#5159) [main] Update dependencies from devdiv/DevDiv/vs-code-coverage
Co-authored-by: Amaury Levé <[email protected]>
…lyzers (microsoft#5168) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…t/arcade (microsoft#5173) [main] Update dependencies from devdiv/DevDiv/vs-code-coverage, dotnet/arcade
…: Build ID 2655117
…: Build ID 2656785
Co-authored-by: Youssef Victor <[email protected]>
Co-authored-by: Amaury Levé <[email protected]>
…t/arcade (microsoft#5296) [main] Update dependencies from devdiv/DevDiv/vs-code-coverage, dotnet/arcade
…: Build ID 2669859
…ETFramework.ReferenceAssemblies (microsoft#5303) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…ETFramework.ReferenceAssemblies (microsoft#5304) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…t/arcade (microsoft#5312) [main] Update dependencies from devdiv/DevDiv/vs-code-coverage, dotnet/arcade
…t/arcade (microsoft#5318) [main] Update dependencies from devdiv/DevDiv/vs-code-coverage, dotnet/arcade
…soft#5322) [main] Update dependencies from devdiv/DevDiv/vs-code-coverage
…t/arcade (microsoft#5326) [main] Update dependencies from devdiv/DevDiv/vs-code-coverage, dotnet/arcade
…: Build ID 2673856
|
Replacing with #5549, to see how far it is from being done. Picked the commits from the start of the PR to avoid stealing code. |
9 participants
No description provided.