Unskip tests with Done issues + dotnet-format test fix #41684

MiYanni · 2024-06-19T20:26:36Z

Summary

Our repo has many skipped tests. I've created a Test Debt label that I used to mark tests that were skipped by linking an associated issue in the Skip text. This label only applies to the issues in this repo.

With all the links (not just SDK repo), I went through and opened the issues to see which ones were Done. The ones that were marked Done I've unskipped in this PR. Additionally, I noticed an issue with the dotnet-format tests, fixed it, and uncommented the skipped job in the CI.

…ticed the pathing issue for dotnet-format, fixed it, and reenabled the CI job.

nagilson

Thank you, this is a wonderful idea and something it'd be great to have a real policy on. (e.g. maybe with the label, a bot pings after 90 days to see if it can be unskipped, or it makes a new issue to unskip it if the other one is closed.)

The changes look good, although unfortunately while marked as done a lot of them are still failing, 33 of them so far. At a glance some of them might be due to breaking changes, or just that they never were fixed. I'm guessing you'd need to go through each test individually and decide what to do again to merge it green, which I'm not sure you want to do atm

test/dotnet-format.Tests/tests/Utilities/TestProjectsPathHelper.cs

MiYanni · 2024-06-20T17:21:27Z

@nagilson

I'm guessing you'd need to go through each test individually and decide what to do again to merge it green, which I'm not sure you want to do atm

Yup, that's exactly what I have to do. It is gonna take a bit of time. Run them locally, take a look if it is something fixable. If not, I'll punt back to the original issue or a new one.

Evangelink

Approving for the dotnet test part. Thanks for uncovering this skipped test.

MiYanni · 2024-06-20T20:57:56Z

test/dotnet-watch.Tests/Watch/ProgramTests.cs

-        [PlatformSpecificFact(TestPlatforms.Windows | TestPlatforms.Linux, Skip = "https://github.com/dotnet/aspnetcore/issues/23394")]
+        [PlatformSpecificFact(TestPlatforms.Windows | TestPlatforms.Linux)]


This unskip did not work. Details explained here: dotnet/aspnetcore#23394 (comment)

marcpopMSFT · 2024-06-20T22:06:23Z

@v-wuzhai Yanni has done some cleanup here for tests that were skipped and linked to issues that had been closed. He also marked any still active issues as related to test debt.

What he didn't do is go through any tests that were marked as skipped but weren't linked to an issue. We think it makes sense to go through those, file issues for them, and make a PR to link them to the issue (if the test is still failing). If the test is no longer failing, we can remove the skip label. Do you think you can do that? No rush but seems worth doing to clean up how we're tracking failing tests.

MiYanni · 2024-06-21T20:43:11Z

test/Microsoft.TemplateEngine.Cli.UnitTests/ParserTests/TabCompletionTests.cs

-#pragma warning disable xUnit1004 // Test methods should not be skipped
-        [Fact(Skip = "https://github.com/dotnet/templating/issues/4192")]
-#pragma warning restore xUnit1004 // Test methods should not be skipped
+        [Fact]


The test still fails with:

Assert.DoesNotContain() Failure: Item found in collection ↓ (pos 0) Collection: ["--test", "--testChoice"] Found: "--test"

The issue originally linked here was closed. However, it was closed in favor of a different issue. Therefore, I'm just going to replace the Skip message with the new issue.

MiYanni · 2024-06-21T21:12:11Z

.../Microsoft.NET.ToolPack.Tests/GivenThatWeWantToPackAToolProjectWithGeneratePackageOnBuild.cs

-        [Theory(Skip = "https://github.com/dotnet/sdk/issues/3471")]
+        [Theory]


The 2 test cases with generatePackageOnBuild: true fail because of reasons similar to:

C:\Code\sdk\artifacts\bin\redist\Debug\dotnet\sdk\9.0.100-ci\Sdks\NuGet.Build.Tasks.Pack\build\NuGet.Build.Tasks.Pack.targets(221,5): error NU5026: The file 'C:\Code\sdk\artifacts\tmp\Debug\It_packs_succ---74529E27\bin\Debug\net9.0\HelloWorld.runtimeconfig.json' to be packed was not found on disk. [C:\Code\sdk\artifacts\tmp\Debug\It_packs_succ---74529E27\HelloWorld.csproj]

Seems like the test project it builds isn't building properly, and it goes to pack the project, but fails. The original issue was actually marked as a dup of another issue, so I'll put that issue as the Skip reason instead.

MiYanni · 2024-06-21T21:35:54Z

test/Microsoft.NET.Build.Tests/WorkloadTests.cs

-        [Theory(Skip = "https://github.com/dotnet/installer/issues/13361")]
+        [Theory]


Unskipping this test failed. Details here: dotnet/installer#13361 (comment)

MiYanni · 2024-06-21T21:51:35Z

test/Microsoft.NET.Build.Tests/GivenThatWeWantToBuildADesktopExe.cs

-        [FullMSBuildOnlyTheory(Skip = "https://github.com/NuGet/Home/issues/8238")]
+        [FullMSBuildOnlyTheory]


This test still fails. When it tries to build the test project, it errors with:

C:\h\w\BA370A1F\t\dotnetSdkTests\4g0jldi5.t0a\DesktopInBoxC---C644200B\DesktopInBoxConflictResolution\Program.cs(1,14): error CS0430: The extern alias 'snh' was not specified in a /reference option [C:\h\w\BA370A1F\t\dotnetSdkTests\4g0jldi5.t0a\DesktopInBoxC---C644200B\DesktopInBoxConflictResolution\DesktopInBoxConflictResolution.csproj]

The original issue was closed and copied to a new issue. So, I'll change this Skip text to be against the new issue.

MiYanni · 2024-06-21T22:12:40Z

test/Microsoft.NET.Build.Tests/GivenThatWeWantToBuildACppCliProjectWithTransitiveDeps.cs

-        [FullMSBuildOnlyFact(Skip = "https://github.com/dotnet/sdk/issues/3785")]
+        [FullMSBuildOnlyFact]


Unskipping this test failed. Marking it with the successor issue. Details here: #11008 (comment)

MiYanni · 2024-06-21T22:12:46Z

test/Microsoft.NET.Build.Tests/GivenThatWeWantToBuildACppCliProjectWithTransitiveDeps.cs

-        [FullMSBuildOnlyFact(Skip = "https://github.com/dotnet/sdk/issues/3785")]
+        [FullMSBuildOnlyFact]


Unskipping this test failed. Marking it with the successor issue. Details here: #11008 (comment)

MiYanni · 2024-06-21T22:12:51Z

test/Microsoft.NET.Build.Tests/GivenThatWeWantToBuildACppCliProjectWithTransitiveDeps.cs

-        [FullMSBuildOnlyFact(Skip = "https://github.com/dotnet/sdk/issues/3785")]
+        [FullMSBuildOnlyFact]


Unskipping this test failed. Marking it with the successor issue. Details here: #11008 (comment)

MiYanni · 2024-06-21T22:31:57Z

test/dotnet-sln.Tests/GivenDotnetSlnAdd.cs

-        [Fact(Skip = "https://github.com/dotnet/sdk/issues/522")]
+        [Fact]


Unskipping failed. Details here: #522 (comment)

MiYanni · 2024-06-21T22:45:47Z

test/Microsoft.DotNet.GenAPI.Tests/CSharpFileBuilderTests.cs

-        [Fact(Skip = "https://github.com/dotnet/roslyn/issues/67019")]
+        [Fact]


Unskipping failed. Could not reopen original issue. New issue with details here: dotnet/roslyn#74109

MiYanni · 2024-06-21T22:45:51Z

test/Microsoft.DotNet.GenAPI.Tests/CSharpFileBuilderTests.cs

-        [Fact(Skip = "https://github.com/dotnet/roslyn/issues/67019")]
+        [Fact]


Unskipping failed. Could not reopen original issue. New issue with details here: dotnet/roslyn#74109

… skipped with new issue numbers.

…erWasNotInitialized test since it was removed but a bad branch merge reintroduced it. Set DependentUponTest as WindowsOnlyTheory since it literally runs the EXE it produces during the test.

MiYanni · 2024-07-01T22:57:22Z

test/dotnet-test.Tests/GivenDotnetTestBuildsAndRunsTestfromCsproj.cs

-            // Call test
-            CommandResult result = new DotnetTestCommand(Log, disableNewOutput: true)
-                                        .WithWorkingDirectory(testProjectDirectory)
-                                        .Execute(
-                                            "--collect", "Code Coverage",
-                                            "--filter", "VSTestPassTest");
-
-            // Verify test results
-            if (!TestContext.IsLocalized())
-            {
-                result.StdOut.Should().Contain("No code coverage data available. Code coverage is currently supported only on Windows and Linux x64.");
-                result.StdOut.Should().Contain("Total:     1");
-                result.StdOut.Should().Contain("Passed:     1");
-                result.StdOut.Should().NotContain("Failed!");
-            }
-
-            result.ExitCode.Should().Be(0);
-        }
-
-        [PlatformSpecificFact(TestPlatforms.Linux, Skip = "https://github.com/dotnet/sdk/issues/22865")]
-        public void ItShouldShowWarningMessageOnCollectCodeCoverageThatProfilerWasNotInitialized()
-        {
-            var testProjectDirectory = CopyAndRestoreVSTestDotNetCoreTestApp("13");
-


This test was removed from main here. The same test was also removed from 6.0.2xx here. Then, the branch merge from 6.0.2xx into main added it back. This was likely a merge issue and the test was intended to be removed, since the first PR was indicated as the reason #22865 could be resolved. So, we're rightfully removing the test from main.

Removed Skip on tests that linked to issues that were marked Done. No…

880e096

…ticed the pathing issue for dotnet-format, fixed it, and reenabled the CI job.

MiYanni requested a review from a team June 19, 2024 20:26

MiYanni requested review from a team, tmat and arunchndr as code owners June 19, 2024 20:26

dotnet-issue-labeler bot added Area-Infrastructure untriaged Request triage from a team member labels Jun 19, 2024

This was referenced Jun 19, 2024

dotnet-watch tests failing #40006

Open

Darwin Build_Release failure on Helix tests - Tests timing out #40074

Open

nagilson reviewed Jun 20, 2024

View reviewed changes

test/dotnet-format.Tests/tests/Utilities/TestProjectsPathHelper.cs Show resolved Hide resolved

Evangelink approved these changes Jun 20, 2024

View reviewed changes

MiYanni commented Jun 20, 2024

View reviewed changes

MiYanni commented Jun 21, 2024

View reviewed changes

Reskipping many tests as most of the failures still exist. Some tests…

3f1e67e

… skipped with new issue numbers.

build-analysis bot mentioned this pull request Jun 22, 2024

The Operation will be canceled. The next steps may not contain expected logs. dotnet/dnceng#3008

Open

3 tasks

MiYanni mentioned this pull request Jul 1, 2024

Microsoft.DotNet.GenAPI.Tests/CSharpFileBuilderTests.cs in SDK repo test failures dotnet/roslyn#74109

Open

Removed the ItShouldShowWarningMessageOnCollectCodeCoverageThatProfil…

09d9fc0

…erWasNotInitialized test since it was removed but a bad branch merge reintroduced it. Set DependentUponTest as WindowsOnlyTheory since it literally runs the EXE it produces during the test.

MiYanni commented Jul 1, 2024

View reviewed changes

Merge branch 'main' into UnskipTestsWithDoneIssues

5d6326a

build-analysis bot mentioned this pull request Jul 2, 2024

VMR CI build failures during restore: feed download failed #41590

Open

MiYanni merged commit 962fbf2 into dotnet:main Jul 2, 2024
37 checks passed

MiYanni deleted the UnskipTestsWithDoneIssues branch July 2, 2024 16:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unskip tests with Done issues + dotnet-format test fix #41684

Unskip tests with Done issues + dotnet-format test fix #41684

MiYanni commented Jun 19, 2024 •

edited

Loading

nagilson left a comment

MiYanni commented Jun 20, 2024

Evangelink left a comment

MiYanni Jun 20, 2024

marcpopMSFT commented Jun 20, 2024

MiYanni Jun 21, 2024

MiYanni Jun 21, 2024

MiYanni Jun 21, 2024 •

edited

Loading

MiYanni Jun 21, 2024

MiYanni Jun 21, 2024

MiYanni Jun 21, 2024

MiYanni Jun 21, 2024

MiYanni Jun 21, 2024

MiYanni Jun 21, 2024

MiYanni Jun 21, 2024

MiYanni Jul 1, 2024

		[PlatformSpecificFact(TestPlatforms.Windows \| TestPlatforms.Linux, Skip = "https://github.com/dotnet/aspnetcore/issues/23394")]
		[PlatformSpecificFact(TestPlatforms.Windows \| TestPlatforms.Linux)]

		[Theory(Skip = "https://github.com/dotnet/sdk/issues/3471")]
		[Theory]

		[FullMSBuildOnlyTheory(Skip = "https://github.com/NuGet/Home/issues/8238")]
		[FullMSBuildOnlyTheory]

		[FullMSBuildOnlyFact(Skip = "https://github.com/dotnet/sdk/issues/3785")]
		[FullMSBuildOnlyFact]

		[Fact(Skip = "https://github.com/dotnet/sdk/issues/522")]
		[Fact]

Unskip tests with Done issues + dotnet-format test fix #41684

Unskip tests with Done issues + dotnet-format test fix #41684

Conversation

MiYanni commented Jun 19, 2024 • edited Loading

Summary

nagilson left a comment

Choose a reason for hiding this comment

MiYanni commented Jun 20, 2024

Evangelink left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcpopMSFT commented Jun 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MiYanni Jun 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MiYanni commented Jun 19, 2024 •

edited

Loading

MiYanni Jun 21, 2024 •

edited

Loading