-
Notifications
You must be signed in to change notification settings - Fork 4.9k
Conversation
Flagged as [WIP] until the dependency change goes into coreclr |
AbandonedMutexException ame = Assert.Throws<AbandonedMutexException>(() => WaitHandle.WaitAny(new[] { m }, FailedWaitTimeout)); | ||
Assert.Equal(0, ame.MutexIndex); | ||
Assert.Equal(m, ame.Mutex); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might consider refactoring this a bit, e.g. (pseudo-code)
public static IEnumerable<object[]> AbandonExisting_MemberData()
{
for (int waitType = 0; waitType < 2; waitType++) // 0 == Wait, 1 == WaitAny
{
yield return new object[] { null, waitType};
yield return new object[] { Guid.NewGuid().ToString("N"), waitType };
}
}
[Theory]
[MemberData(nameof(AbandonExisting_MemberData))]
public void AbandonExisting(string name, int waitType)
{
using (Mutex m = new Mutex(false, name))
{
Task.Factory.StartNew(() => Assert.True(m.WaitOne(FailedWaitTimeout),
CancellationToken.None, TaskCreationOptions.LongRunning, TaskScheduler.Default).Wait();
switch (waitType)
{
case 0:
Assert.Throws<AbandonedMutexException>(() => m.WaitOne(FailedWaitTimeout));
break;
case 1:
AbandonedMutexException ame = Assert.Throws<AbandonedMutexException>(() => WaitHandle.WaitAny(new[] { m }, FailedWaitTimeout));
Assert.Equal(0, ame.MutexIndex);
Assert.Equal(m, ame.Mutex);
break;
}
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Thanks for updating these (and for the Mutex implementation!) One comment, otherwise LGTM. We should also look at adding a cross-process test of the naming, similar in concept to https://github.com/dotnet/corefx/blob/master/src/System.Threading/tests/SemaphoreTests.cs#L281-L317 . That could be done as a separate PR if you wanted, though it might be useful here as well to help validate the CoreCLR change. |
I'm doing all of the cross-process testing in the PAL tests, and leaving the CoreFX tests to just do basic API-level testing (to cover the layer from the .NET API to the PAL Windows API). I'll take a closer look at the test you pointed to, and will see if I can add something similar. |
The tests removed would fail soon after dotnet/coreclr#5030 is merged. Related to dotnet/coreclr#5030 Related to dotnet#8625 Related to dotnet/coreclr#3422
Looked at the test, I have tests like that in PAL tests already |
b25405a
to
e68b52c
Compare
@dotnet-bot test this please |
3 similar comments
@dotnet-bot test this please |
@dotnet-bot test this please |
@dotnet-bot test this please |
Related to dotnet/coreclr#5030 Related to dotnet/coreclr#3422
This reverts commit 697bd95. Equivalent to PR dotnet#8625
The tests removed would fail soon after dotnet/coreclrdotnet/corefx#5030 is merged. Related to dotnet/coreclrdotnet/corefx#5030 Related to dotnet/corefx#8625 Related to dotnet/coreclrdotnet/corefx#3422 Commit migrated from dotnet/corefx@4c4924d
Fix some named mutex tests Commit migrated from dotnet/corefx@4f782d4
This reverts commit dotnet/corefx@697bd95. Equivalent to PR dotnet/corefx#8625 Commit migrated from dotnet/corefx@d39cc2f
Related to dotnet/coreclr#5030
Related to dotnet/coreclr#3422