Test re-enabling SRC cache tests. #117898
Conversation
|
/azp run runtime-linuxbionic |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp list |
|
/azp run runtime-libraries-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-libraries-mono outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-linuxbionic |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-libraries-coreclr outerloop |
|
/azp run runtime-libraries-mono outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
1 similar comment
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-linuxbionic |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-libraries-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-libraries-mono outerloop |
|
/azp run runtime-linuxbionic |
|
Azure Pipelines successfully started running 1 pipeline(s). |
1 similar comment
|
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull Request Overview
This pull request re-enables several System.Runtime.Caching tests that were previously disabled due to various issues. The changes remove test skip attributes and ActiveIssue annotations to allow these tests to run again, with the assumption that the underlying issues have been resolved.
Key changes include:
- Removal of ActiveIssue and platform-specific skip attributes from multiple test methods
- Addition of a new test case for file change monitoring with reasonable delay
- Minor code organization improvements in imports
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| MemoryCacheTest.cs | Removes ActiveIssue and platform skip attributes from Contains, Trim, TestCacheSliding, TestCacheShrink, and TestCacheExpiryOrdering tests |
| HostFileChangeMonitorTest.cs | Removes ActiveIssue attributes from file change monitoring tests, adds new Reasonable_Delay test, and improves SetupMonitoring method to use unique paths |
|
|
||
| private static Tuple<string, string, string, IList<string>> SetupMonitoring() | ||
| private static Tuple<string, string, string, IList<string>> SetupMonitoring(string uniqueId) | ||
| { |
There was a problem hiding this comment.
The SetupMonitoring method signature change from no parameters to requiring a uniqueId parameter is a breaking change for any existing callers. Consider making the uniqueId parameter optional with a default value to maintain backward compatibility.
| { | |
| private static Tuple<string, string, string, IList<string>> SetupMonitoring(string uniqueId = null) | |
| { | |
| if (uniqueId == null) | |
| uniqueId = Guid.NewGuid().ToString("N"); |
| File.WriteAllText(setup.Item2, "I am the first file. Updated."); | ||
|
|
||
| // Wait for the monitor to detect the change - 5s should be more than enough | ||
| for (int i = 0; i < 250; i++) |
There was a problem hiding this comment.
The magic number 250 for loop iterations should be extracted to a named constant to improve code readability and maintainability.
| for (int i = 0; i < 250; i++) | |
| for (int i = 0; i < MaxWaitIterations; i++) |
| { | ||
| if (!mc.Contains("key")) | ||
| break; | ||
| Thread.Sleep(20); |
There was a problem hiding this comment.
The magic number 20 for sleep duration should be extracted to a named constant to improve code readability and maintainability.
| Thread.Sleep(20); | |
| Thread.Sleep(PollingIntervalMilliseconds); |
1 participant
Test re-enabling some long-disabled cache tests that I think are probably ok.