Fix intermittent OSX CI failures in StartsWithNoMatch_StringComparison and EndsWithNoMatch_StringComparison#125963
Fix intermittent OSX CI failures in StartsWithNoMatch_StringComparison and EndsWithNoMatch_StringComparison#125963
Conversation
|
Tagging subscribers to this area: @dotnet/area-infrastructure-libraries |
…put in StartsWithNoMatch_StringComparison Co-authored-by: kotlarmilos <11523312+kotlarmilos@users.noreply.github.com> Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/ebe921f3-5e85-4102-be01-a6bd6fffada3
…improve diagnostic output Co-authored-by: kotlarmilos <11523312+kotlarmilos@users.noreply.github.com> Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/ebe921f3-5e85-4102-be01-a6bd6fffada3
There was a problem hiding this comment.
Pull request overview
Updates StringTests to avoid intermittent macOS test failures caused by non-deterministic Apple collation behavior for certain Unicode inputs, by expanding existing platform guards and improving failure diagnostics.
Changes:
- Add
[ActiveIssue(..., IsOSX)]toEndsWithNoMatch_StringComparisonandStartsWithNoMatch_StringComparisonto disable these tests on macOS (in addition to existing Apple mobile coverage). - Refactor repeated culture-comparison assertions into a loop and add richer diagnostic messages when the assertions fail.
| foreach (StringComparison comp in new[] { StringComparison.CurrentCulture, StringComparison.CurrentCultureIgnoreCase, StringComparison.InvariantCulture, StringComparison.InvariantCultureIgnoreCase }) | ||
| { | ||
| bool expected = s1.ToString().EndsWith(s2.ToString(), comp); | ||
| bool actual = s1.EndsWith(s2, comp); | ||
| Assert.True(expected == actual, |
There was a problem hiding this comment.
Using Assert.True(expected == actual, ...) loses xUnit's default expected/actual diff output. Consider including expected and actual values in the failure message (or otherwise preserving them) so failures are as actionable as before, especially since this PR is focused on improving diagnostics.
| foreach (StringComparison comp in new[] { StringComparison.CurrentCulture, StringComparison.CurrentCultureIgnoreCase, StringComparison.InvariantCulture, StringComparison.InvariantCultureIgnoreCase }) | ||
| { | ||
| bool expected = firstSpan.ToString().EndsWith(secondSpan.ToString(), comp); | ||
| bool actual = firstSpan.EndsWith(secondSpan, comp); | ||
| Assert.True(expected == actual, |
There was a problem hiding this comment.
Using Assert.True(expected == actual, ...) loses xUnit's default expected/actual diff output. Consider including expected and actual values in the failure message (or otherwise preserving them) so failures are as actionable as before, especially since this PR is focused on improving diagnostics.
| foreach (StringComparison comp in new[] { StringComparison.CurrentCulture, StringComparison.CurrentCultureIgnoreCase, StringComparison.InvariantCulture, StringComparison.InvariantCultureIgnoreCase }) | ||
| { | ||
| bool expected = s1.ToString().StartsWith(s2.ToString(), comp); | ||
| bool actual = s1.StartsWith(s2, comp); | ||
| Assert.True(expected == actual, |
There was a problem hiding this comment.
Using Assert.True(expected == actual, ...) loses xUnit's default expected/actual diff output. Consider including expected and actual values in the failure message (or otherwise preserving them) so failures are as actionable as before, especially since this PR is focused on improving diagnostics.
| foreach (StringComparison comp in new[] { StringComparison.CurrentCulture, StringComparison.CurrentCultureIgnoreCase, StringComparison.InvariantCulture, StringComparison.InvariantCultureIgnoreCase }) | ||
| { | ||
| bool expected = firstSpan.ToString().StartsWith(secondSpan.ToString(), comp); | ||
| bool actual = firstSpan.StartsWith(secondSpan, comp); | ||
| Assert.True(expected == actual, |
There was a problem hiding this comment.
Using Assert.True(expected == actual, ...) loses xUnit's default expected/actual diff output. Consider including expected and actual values in the failure message (or otherwise preserving them) so failures are as actionable as before, especially since this PR is focused on improving diagnostics.
|
@copilot address this |
…EndsWith assertions Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/dd927ad2-980d-4369-895a-0a96fe17f1ca Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com>
Apple's
NSString compare:options:range:locale:is non-deterministic for certain Unicode inputs (C1 control chars\u0080–\u009F), causingStartsWithNoMatch_StringComparisonandEndsWithNoMatch_StringComparisonto fail intermittently on macOS. The existing[ActiveIssue]only coveredIsAppleMobile(iOS/tvOS), leaving macOS unguarded.Changes
StartsWithNoMatch_StringComparison: Add[ActiveIssue(..., IsOSX)]referencing issue [Apple-mobile]System.Tests.StringTests.[Starts,Ends]WithNoMatch_StringComparisontest failure #108832; replace four separateAssert.Equalcalls with aforeachloop usingAssert.True(expected == actual, message)that captureslength,mismatchIndex,comp, char values at the mismatch index, and theexpected/actualboolean values for easier debugging when re-enabled.EndsWithNoMatch_StringComparison: Same fix — same root cause (same Apple native API path), same[ActiveIssue]gap, same diagnostic improvement applied consistently.The diagnostic message format:
Original prompt
This section details on the original issue you should resolve
<issue_title>OSX failure on StringTests.StartsWithNoMatch_StringComparison</issue_title>
<issue_description>## Build Information
Build: https://dev.azure.com/dnceng-public/cbb18261-c48f-4abb-8651-8cdcb5474649/_build/results?buildId=940013
Build error leg or test failing: System.Tests.StringTests.StartsWithNoMatch_StringComparison
Pull request: #111877
Error Message
Fill the error message using step by step known issues guidance.
{ "ErrorMessage": "[FAIL] System.Tests.StringTests.StartsWithNoMatch_StringComparison", "ErrorPattern": "", "BuildRetry": false, "ExcludeConsoleLog": false }Log
Known issue validation
Build: 🔎 https://dev.azure.com/dnceng-public/public/_build/results?buildId=940013
Error message validated:
[[FAIL] System.Tests.StringTests.StartsWithNoMatch_StringComparison]Result validation: ❌ Known issue did not match with the provided build.
Validation performed at: 2/5/2025 4:19:32 PM UTC
Report
Summary
Comments on the Issue (you are @copilot in this section)
@tarekgh @matouskozak please triage. Thanks! @matouskozak Looks like duplicate of https://github.com//issues/108832 but with slightly different error message. fyi: @kotlarmilos @danmoseley > [!NOTE] > This analysis was generated by GitHub Copilot.Analysis: Root cause identifiable from code inspection
A dump or repro isn't strictly necessary here — this is a wrong-return-value bug, not a crash, so a dump wouldn't add much. The root cause is identifiable from the code.
What the test does
The test (
StartsWithNoMatch_StringComparison) creates strings from chars\u0001–\u0096(control characters + basic Latin), introduces a single-char mismatch at each position, then asserts that culture-awareStartsWithreturns consistent results between calls:The failure (
Expected: True, Actual: False) means the same underlying comparison returns different results on different invocations for the same input.Where it goes wrong
On Apple mobile in hybrid globalization mode, culture-aware
StartsWithroutes throughGlobalizationNative_StartsWithNativeinpal_collation.m: