Skip to content

Re-enable tests for Sys.RT.Caching #118508

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
StephenMolloy wants to merge 4 commits into from
Closed

Re-enable tests for Sys.RT.Caching #118508

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2dda943
Test re-enabling SRC cache tests.
StephenMolloy Jul 21, 2025
20083f5
Enable some more tests. Add one for file change monitoring.
StephenMolloy Jul 23, 2025
cb66180
Allow a little more time on timing test for robustness.
StephenMolloy Jul 23, 2025
eb9f506
Cleanup temp-comments; re-enable outerloop
StephenMolloy Aug 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 47 additions & 8 deletions ...ibraries/System.Runtime.Caching/tests/System.Runtime.Caching/HostFileChangeMonitorTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,15 @@

using System;
using System.Collections.Generic;
using System.Collections.Specialized;
using System.IO;
using System.Reflection;
using System.Runtime.Caching;
using System.Runtime.Caching.Hosting;
using System.Text;

using Xunit;
using System.Threading;
using MonoTests.Common;
using Xunit;

namespace MonoTests.System.Runtime.Caching
{
Expand Down Expand Up @@ -84,7 +85,6 @@ public void Constructor_Exceptions()
}

[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/34497", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)]
public static void Constructor_MissingFiles_Handler()
{
HostFileChangeMonitor monitor;
Expand Down Expand Up @@ -132,7 +132,6 @@ public static void Constructor_MissingFiles_Handler()
}

[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/34497", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)]
public void Constructor_Duplicates()
{
HostFileChangeMonitor monitor;
Expand All @@ -148,9 +147,9 @@ public void Constructor_Duplicates()
monitor.Dispose();
}

private static Tuple<string, string, string, IList<string>> SetupMonitoring()
private static Tuple<string, string, string, IList<string>> SetupMonitoring(string uniqueId)
{
string testPath = Path.Combine(Path.GetTempPath(), "HostFileChangeMonitorTest", "Dispose_Calls_StopMonitoring");
string testPath = Path.Combine(Path.GetTempPath(), "HostFileChangeMonitorTest", uniqueId);
if (!Directory.Exists(testPath))
Directory.CreateDirectory(testPath);

Expand Down Expand Up @@ -201,13 +200,12 @@ private static void CleanupMonitoring(Tuple<string, string, string, IList<string
}

[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/34497", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)]
public void UniqueId()
{
Tuple<string, string, string, IList<string>> setup = null;
try
{
setup = SetupMonitoring();
setup = SetupMonitoring(nameof(UniqueId));
FileInfo fi;
var monitor = new HostFileChangeMonitor(setup.Item4);
var sb = new StringBuilder();
Expand Down Expand Up @@ -247,5 +245,46 @@ public void UniqueId()
CleanupMonitoring(setup);
}
}

[OuterLoop]
[Fact]
public void Reasonable_Delay()
{
Tuple<string, string, string, IList<string>> setup = null;
try
{
setup = SetupMonitoring(nameof(Reasonable_Delay));

using var monitor = new HostFileChangeMonitor(setup.Item4);
var policy = new CacheItemPolicy
{
ChangeMonitors = { monitor }
};
var config = new NameValueCollection();
var mc = new PokerMemoryCache("MyCache", config);

mc.Set("key", "value", policy);

// Verify the cache item is set
Assert.Equal("value", mc["key"]);

// Update the file dependency
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++)
{
if (!mc.Contains("key"))
break;
Thread.Sleep(20);
Comment thread
StephenMolloy marked this conversation as resolved.
Comment thread
StephenMolloy marked this conversation as resolved.
}
Copy link
Copy Markdown
Member

@mconnew mconnew Aug 8, 2025

Choose a reason for hiding this comment

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

When there are a lot of tests running in parallel (which xunit attempts to achieve), there can be a lot of thread contention and this might not return within 20ms. I would create a Stopwatch before the loop and use a while loop checking if the required amount of time has elapsed yet. This would be immune to CPU contention causing the test to take significantly longer due to the iteration count.

Copy link
Copy Markdown
Member Author

@StephenMolloy StephenMolloy Aug 9, 2025

Choose a reason for hiding this comment

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

I thought about that. Decided to just keep it simple though. The exact timing isn't important. The cache item should be removed relatively quickly - just not instantly, which is why we check on it in a loop. I'd be shocked if it ever took longer than 1s, but I wanted to be flexible enough so the test isn't noisy. 5s felt like an eternity for this, but in reality, the condition for breaking the loop should be met much, much sooner... and what that exact time is isn't important.

Do you think the potential for running so much longer than 5s that it becomes a problem in the test harness is a realistic problem?

Copy link
Copy Markdown
Member Author

@StephenMolloy StephenMolloy Aug 9, 2025

Choose a reason for hiding this comment

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

Ooh! Also, I switched from Thread.Sleep() to Task.Delay() and re-created the PR instead of mucking around with some re-basing/conflict resolution... and proceeded to use the old branch again. :/

I'm going to close this PR and assign the new one for the correct branch to you. It's all the same changes except with one more commit updating to Task.Delay().


Assert.Null(mc["key"]);
}
finally
{
CleanupMonitoring(setup);
}
}
}
}
12 changes: 1 addition & 11 deletions src/libraries/System.Runtime.Caching/tests/System.Runtime.Caching/MemoryCacheTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,6 @@ public void Indexer(string throwOnDisposed)
}

[Theory, InlineData("true"), InlineData("false"), InlineData(null)]
//[ActiveIssue("https://github.com/dotnet/runtime/issues/1429")]
public void Contains(string throwOnDisposed)
{
var mc = CreatePokerMemoryCache("MyCache", throwOnDisposed);
Expand Down Expand Up @@ -1109,9 +1108,7 @@ public void ChangeMonitors()
Assert.False(onChangedCalled);
}

// Due to internal implementation details Trim has very few easily verifiable scenarios
// ActiveIssue: https://github.com/dotnet/runtime/issues/36488
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotArm64Process))]
[Theory]
[InlineData("true"), InlineData("false"), InlineData(null)]
public void Trim(string throwOnDisposed)
{
Expand Down Expand Up @@ -1197,11 +1194,6 @@ public void TestExpiredGetValues()

[OuterLoop] // makes long wait
[Fact]
// This little dance is needed to prevent this test from running against the OS-specific
// runtime binary on the wrong OS. Without it, this test will run for each 'TargetFramework'
// in the test csproj, and the non-windows framework will run against the non-windows library
// because 'netstandard' is still valid for windows execution.
[PlatformSpecific(TestPlatforms.Windows)]
public void TestCacheSliding()
{
var config = new NameValueCollection();
Expand Down Expand Up @@ -1510,7 +1502,6 @@ public class MemoryCacheTestExpires4
public static bool SupportsPhysicalMemoryMonitor => MemoryCacheTest.SupportsPhysicalMemoryMonitor;

[ConditionalFact(nameof(SupportsPhysicalMemoryMonitor))]
[SkipOnPlatform(TestPlatforms.LinuxBionic, "https://github.com/dotnet/runtime/issues/93106")]
public async Task TestCacheShrink()
{
const int HEAP_RESIZE_THRESHOLD = 8192 + 2;
Expand Down Expand Up @@ -1570,7 +1561,6 @@ public class MemoryCacheTestExpires5
public static bool SupportsPhysicalMemoryMonitor => MemoryCacheTest.SupportsPhysicalMemoryMonitor;

[ConditionalFact(nameof(SupportsPhysicalMemoryMonitor))]
[SkipOnPlatform(TestPlatforms.LinuxBionic, "https://github.com/dotnet/runtime/issues/93106")]
public async Task TestCacheExpiryOrdering()
{
var config = new NameValueCollection();
Expand Down
Loading