Skip to content

Conversation

SimonZhao888
Copy link
Member

@SimonZhao888
Copy link
Member Author

@tmat, are we allowed to use Task.Delay() method in test case to enhance the stability of test case?

@tmat
Copy link
Member

tmat commented Jul 7, 2025

@SimonZhao888 Please, no task delays.

Copy link
Member

@tmat tmat left a comment

Choose a reason for hiding this comment

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

This is not appropriate way of fixing tests.

Simon Zhao (BEYONDSOFT CONSULTING INC) added 2 commits July 16, 2025 17:02
@SimonZhao888 SimonZhao888 marked this pull request as ready for review July 17, 2025 08:28
@SimonZhao888 SimonZhao888 requested a review from a team as a code owner July 17, 2025 08:28
@SimonZhao888 SimonZhao888 added the Area-Containers Related to dotnet SDK containers functionality label Jul 17, 2025
@Copilot Copilot AI review requested due to automatic review settings August 4, 2025 06:20
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR re-enables previously skipped tests in the DotNet.Watcher.Tests project by removing the Skip parameter that was referencing issue #42921. The tests were temporarily disabled and are now being brought back into the active test suite.

  • Removes Skip attributes from two test methods that were disabled due to issue #42921
  • Re-enables ChangeCompiledFile and ChangeExcludedFile tests in the globbing app test suite

@@ -12,7 +12,7 @@ public GlobbingAppTests(ITestOutputHelper logger)
{
}

[ConditionalTheory(Skip = "https://github.com/dotnet/sdk/issues/42921")]
[ConditionalTheory]
Copy link
Preview

Copilot AI Aug 4, 2025

Choose a reason for hiding this comment

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

Re-enabling this test should include verification that the underlying issue #42921 has been resolved. Consider adding a comment explaining why the test is now safe to re-enable or what changes fixed the original problem.

Copilot generated this review using guidance from copilot-instructions.md.

@@ -85,7 +85,7 @@ public async Task RenameCompiledFile()
await App.AssertStarted();
}

[Fact(Skip = "https://github.com/dotnet/sdk/issues/42921")]
[Fact]
Copy link
Preview

Copilot AI Aug 4, 2025

Choose a reason for hiding this comment

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

Re-enabling this test should include verification that the underlying issue #42921 has been resolved. Consider adding a comment explaining why the test is now safe to re-enable or what changes fixed the original problem.

Copilot generated this review using guidance from copilot-instructions.md.

@SimonZhao888 SimonZhao888 added Test Debt and removed Area-Containers Related to dotnet SDK containers functionality labels Aug 4, 2025
@nagilson nagilson merged commit 0caf101 into dotnet:main Aug 4, 2025
29 checks passed
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.

4 participants