Add ms:format-date and ms:format-time test cases #113507
Conversation
|
When this is landed, we need to include in PR #97402 an update to the expected values in bug93189.xml test data out/dates/test3 to be |
|
Updated the XmlDiff classes to handle space-normalization (e.g. various things like non-breaking space, zero-width space, etc.) to normal space to ease test case baseline authoring. This is due to the fact that on the test environment, the time format is AM/PM, and turns out that the separator between the minutes and the AM/PM is not a space, it's actually a n U+202F NARROW NO-BREAK SPACE so the emitted actual wouldn't match the expected unless you knew to paste that exact character in. |
Test cases for regression bug dotnet#93189 (difference between 4.52 and 6.0) NOTE: The baseline\bug9389.xml reflects the _current_ (broken) behavior so these tests will currently be PASSING. When the bug is resolved, this test data needs to be updated to reflect what _actually_ should happen. Fix problem with writing out the diff.xml file when test fails You have to open the file in Create mode with Write access. Turns out that the separator between the minutes and the AM/PM is not a space, it's actually a n U+202F NARROW NO-BREAK SPACE so update the expected value. Add code to normalize spaces in the XmlDiff. Since it's hard to see different kinds of spaces, add the option to treat them all as normal spaces.
|
Squashed and ready for review |
|
@danmoseley I added a test for PR #97402 per your request |
|
Tagging folks that might want to review this since the auto-tagger didn't. XML folks @jeffhandley @StephenMolloy @HongGit |
| return value; | ||
|
|
||
| if (NormalizeSpaces) | ||
| value = Regex.Replace(value, "[\u00A0\u180E\u2000-\u200B\u202F\u205F\u3000\uFEFF]", " "); |
There was a problem hiding this comment.
From where did you get this list?
\u180E is a format character and not really a space. \uFEFF similar. And missing \u1680.
There was a problem hiding this comment.
The list I used is Unicode spaces pointed at by this answer and it is based on the Unicode category Zs.
It adds \FEFF is zero-width no-break space and seems to be missed by a lot of folks.
It adds \u180E Mongolian vowel separator because it is treated as a space, happy to remove it.
I removed \u1680-Ogham space mark because its glyph is usually rendered as a dash, could easily put it back.
Happy to adjust this pattern as desired.
There was a problem hiding this comment.
I am fine with what you are suggesting here as we are adding this to the tests. What is the XML specs mention in general about the spaces? In general I expect the space character should have Unicode Category as SpaceSeparator.
There was a problem hiding this comment.
It's not entirely about XML's view of spaces, this whole change is motivated because in the test-environment's locale the format for times is h:mm AM except the space isn't actually a space but is really the \u202F Narrow No-Break Space which broke my naive test. When the need for a normalization to spaces (to defend against locale-driven possibilities) I figured I should cast a slightly wider net.
There's absolutely no objection in my mind to just using \s in the pattern since that only result in ignoring a few more "blanks" and this is nothing but test code anyway. Point me and I'll shoot as ordered :)
There was a problem hiding this comment.
In the date/time parsing code we just call char.IsWhiteSpace. Either you continue using regex as you suggested or just manually create a helper method iterate over the string checking using char.IsWhiteSpace. What ever you feel easier to you, do it :-)
Thanks for your help fixing the tests!
There was a problem hiding this comment.
one note, if you use the regex, most likely you need \p{Zs} instead of \s. I don't recall if \s cover all different whitespaces or not.
src/libraries/Common/tests/System/Xml/XmlDiff/XmlDiffDocument.cs
Outdated
Show resolved
Hide resolved
src/libraries/Common/tests/System/Xml/XmlDiff/XmlDiffDocument.cs
Outdated
Show resolved
Hide resolved
src/libraries/Common/tests/System/Xml/XmlDiff/XmlDiffDocument.cs
Outdated
Show resolved
Hide resolved
There was a trailing space that VSCode stripped while I churning on this (ended up not making any other changes to this files). Would you like me to take it out of the commits? |
That is fine to keep it. I was only wondering as was not seeing real changes. |
Moved the space-replacements pattern to XmlDiffOption. Lowercased the constructor arguments for XmlDiffCharacterData. Make XmlDiffProcessingInstruction pass options as named parameters.
It's probably that you have the Hide whitespace changes option toggled at the top of your review window :) |
|
/ba-g the build failure is unrelated |
Test cases for regression bug #93189 (difference between 4.52 and 6.0)
Currently set to PASS with the new (broken) values emitted by .Net 6.0+
When #97402 is landed, we need to update the expected values per this comment