Review skipped test case for regex scanner with \c[ pattern

[jeffhandley]

Cleanup Issue-URLs in Code · Issue #63902 · dotnet/runtime identified a test case that is being skipped, but the referenced issue has been fixed. The PR with the fix indicates that a different approach might have been possible, but the applied fix has been accepted for some time now.

• The original issue was: Regex fails to accept pattern that it probably should. · Issue #24759 · dotnet/runtime
• The PR that applied the fix was: Fix regex scanner for \c[ by danmoseley · Pull Request #26828 · dotnet/corefx

runtime/src/libraries/System.Text.RegularExpressions/tests/Regex.Groups.Tests.cs

Lines 391 to 395 in f04a242

	yield return new object[] { engine, null, @"(cat)(\cz*)(dog)", "asdlkcat\u001adogiwod", RegexOptions.None, new string[] { "cat\u001adog", "cat", "\u001a", "dog" } };
	if (!PlatformDetection.IsNetFramework) // missing fix for https://github.com/dotnet/runtime/issues/24759
	{
	yield return new object[] { engine, null, @"(cat)(\c[*)(dog)", "asdlkcat\u001bdogiwod", RegexOptions.None, new string[] { "cat\u001bdog", "cat", "\u001b", "dog" } };

The skipped test case should be evaluated to determine if the fix made allows the test to pass. With the findings we should either enable the test case or update the code comment to reference this open issue for tracking.

/cc @deeprobin

[ghost]

Tagging subscribers to this area: @dotnet/area-system-text-regularexpressions
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Cleanup Issue-URLs in Code · Issue #63902 · dotnet/runtime identified a test case that is being skipped, but the referenced issue has been fixed. The PR with the fix indicates that a different approach might have been possible, but the applied fix has been accepted for some time now.

• The original issue was: Regex fails to accept pattern that it probably should. · Issue #24759 · dotnet/runtime
• The PR that applied the fix was: Fix regex scanner for \c[ by danmoseley · Pull Request #26828 · dotnet/corefx

runtime/src/libraries/System.Text.RegularExpressions/tests/Regex.Groups.Tests.cs

Lines 391 to 395 in f04a242

	yield return new object[] { engine, null, @"(cat)(\cz*)(dog)", "asdlkcat\u001adogiwod", RegexOptions.None, new string[] { "cat\u001adog", "cat", "\u001a", "dog" } };
	if (!PlatformDetection.IsNetFramework) // missing fix for https://github.com/dotnet/runtime/issues/24759
	{
	yield return new object[] { engine, null, @"(cat)(\c[*)(dog)", "asdlkcat\u001bdogiwod", RegexOptions.None, new string[] { "cat\u001bdog", "cat", "\u001b", "dog" } };

The skipped test case should be evaluated to determine if the fix made allows the test to pass. With the findings we should either enable the test case or update the code comment to reference this open issue for tracking.

/cc @deeprobin

Author:	jeffhandley
Assignees:	-
Labels:	area-System.Text.RegularExpressions, disabled-test
Milestone:	-

[joperezr]

Thanks for opening this @jeffhandley . By quickly skimming the code, seems like the issue was fixed in .Net (Core) but the comment indicates that the test case is skipped in .net framework since the fix wouldn’t be present there. I will still check to see if we can add the testcase, but from the comment it seems like we are skipping the test by design.

[jeffhandley]

Thanks, @joperezr; that makes sense. I can update the comment as part of #65932 to address this then.