Add ms:format-date and ms:format-time test cases by IDisposable · Pull Request #113507 · dotnet/runtime

IDisposable · 2025-03-14T00:20:54Z

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

IDisposable · 2025-03-14T03:59:33Z

When this is landed, we need to include in PR #97402 an update to the expected values in bug93189.xml test data
needs be changed:

out/dates/test3 to be 06/15/2009
out/times/test3 to be 1:45 PM // important, that's not a space, that's a U-202F NARROW NO-BREAK SPACE

IDisposable · 2025-03-14T17:26:08Z

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.

IDisposable · 2025-03-14T20:15:00Z

Squashed and ready for review

IDisposable · 2025-03-18T06:04:50Z

@danmoseley I added a test for PR #97402 per your request
@eiriktsarpalis and yours

IDisposable · 2025-03-18T06:07:14Z

Tagging folks that might want to review this since the auto-tagger didn't.

XML folks @jeffhandley @StephenMolloy @HongGit

PR #97402 reviewers @tarekgh @krwq

tarekgh · 2025-03-18T15:27:04Z

src/libraries/Common/tests/System/Xml/XmlDiff/XmlDiff.cs

+                return value;
+
+            if (NormalizeSpaces)
+                value = Regex.Replace(value, "[\u00A0\u180E\u2000-\u200B\u202F\u205F\u3000\uFEFF]", " ");


"[\u00A0\u180E\u2000-\u200B\u202F\u205F\u3000\uFEFF]"

From where did you get this list?

\u180E is a format character and not really a space. \uFEFF similar. And missing \u1680.

https://unicodeplus.com/category/Zs

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.

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.

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 :)

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!

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

tarekgh · 2025-03-18T15:34:36Z

// The .NET Foundation licenses this file to you under the MIT license.

not seeing any change in this file

Refers to: src/libraries/System.Private.Xml/tests/Xslt/XslTransformApi/CXslTransform.cs:2 in 5dd8d51. [](commit_id = 5dd8d51, deletion_comment = False)

src/libraries/Common/tests/System/Xml/XmlDiff/XmlDiffDocument.cs

tarekgh

Left minor comments. LGTM, otherwise.

@krwq could you have a quick look too?

IDisposable · 2025-03-18T20:37:32Z

// The .NET Foundation licenses this file to you under the MIT license.

not seeing any change in this file

Refers to: src/libraries/System.Private.Xml/tests/Xslt/XslTransformApi/CXslTransform.cs:2 in [5dd8d51]

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?

https://github.com/dotnet/runtime/pull/113507/files#diff-4a94ae4a2848c89aa86609a761b9384b64e292a1f48c3218d0a6ec384feae87cL904

tarekgh · 2025-03-18T21:26:33Z

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.

IDisposable · 2025-03-18T22:41:38Z

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.

It's probably that you have the Hide whitespace changes option toggled at the top of your review window :)

tarekgh · 2025-03-23T21:35:32Z

/ba-g the build failure is unrelated

ghost added the area-System.Xml label Mar 14, 2025

dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Mar 14, 2025

This was referenced Mar 14, 2025

[wasm][AOT][net8.0] emcc : error - received SIGKILL (-9) #89402

Open

/root/helix/work/correlation/scripts/<hash>/execute.sh: Permission denied dotnet/dnceng#3412

Open

IDisposable changed the title ~~Add ms:format-date and ms:format-time test cases~~ [DRAFT] Add ms:format-date and ms:format-time test cases Mar 14, 2025

This was referenced Mar 14, 2025

XSLT extension methods format-date & format-time both output date & time when using the empty string (default) format #93189

Closed

Proposed fix for issue XSLT extension methods format-date & format-time #97402

Closed

IDisposable force-pushed the xslt-test branch from 06ef9a1 to fa45b74 Compare March 14, 2025 04:28

build-analysis bot mentioned this pull request Mar 14, 2025

restarted. Azure DevOps can't recover from restarts. dotnet/dnceng#3879

Closed

3 tasks

IDisposable force-pushed the xslt-test branch from 5d75921 to 5dd8d51 Compare March 14, 2025 20:14

IDisposable changed the title ~~[DRAFT] Add ms:format-date and ms:format-time test cases~~ Add ms:format-date and ms:format-time test cases Mar 14, 2025

IDisposable marked this pull request as ready for review March 14, 2025 20:16

This was referenced Mar 14, 2025

[QUIC & HTTP/3] Handshake Timeout on tests #104426

Closed

System.Net.Quic.Tests failed with "System.Net.Quic.QuicException : The connection timed out from inactivity." #105177

Closed

XHarness message of order error leading to timeout dotnet/dnceng#4823

Closed

tarekgh reviewed Mar 18, 2025

View reviewed changes

src/libraries/Common/tests/System/Xml/XmlDiff/XmlDiffDocument.cs Outdated Show resolved Hide resolved

tarekgh reviewed Mar 18, 2025

View reviewed changes

src/libraries/Common/tests/System/Xml/XmlDiff/XmlDiffDocument.cs Show resolved Hide resolved

tarekgh reviewed Mar 18, 2025

View reviewed changes

src/libraries/Common/tests/System/Xml/XmlDiff/XmlDiffDocument.cs Outdated Show resolved Hide resolved

tarekgh reviewed Mar 18, 2025

View reviewed changes

src/libraries/Common/tests/System/Xml/XmlDiff/XmlDiffDocument.cs Outdated Show resolved Hide resolved

tarekgh approved these changes Mar 18, 2025

View reviewed changes

tarekgh added this to the 10.0.0 milestone Mar 18, 2025

PR feedback

ee45af0

Moved the space-replacements pattern to XmlDiffOption. Lowercased the constructor arguments for XmlDiffCharacterData. Make XmlDiffProcessingInstruction pass options as named parameters.

IDisposable force-pushed the xslt-test branch from 546acf4 to ee45af0 Compare March 18, 2025 22:08

tarekgh merged commit c381503 into dotnet:main Mar 23, 2025
79 of 83 checks passed

IDisposable deleted the xslt-test branch March 25, 2025 05:07

github-actions bot locked and limited conversation to collaborators Apr 24, 2025

Conversation

IDisposable commented Mar 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

IDisposable commented Mar 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

IDisposable commented Mar 14, 2025

Uh oh!

IDisposable commented Mar 14, 2025

Uh oh!

IDisposable commented Mar 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

IDisposable commented Mar 18, 2025

Uh oh!

tarekgh Mar 18, 2025

Choose a reason for hiding this comment

Uh oh!

IDisposable Mar 18, 2025

Choose a reason for hiding this comment

Uh oh!

tarekgh Mar 18, 2025

Choose a reason for hiding this comment

Uh oh!

IDisposable Mar 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tarekgh Mar 18, 2025

Choose a reason for hiding this comment

Uh oh!

tarekgh Mar 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tarekgh commented Mar 18, 2025

Uh oh!

Uh oh!

Uh oh!

tarekgh left a comment

Choose a reason for hiding this comment

Uh oh!

IDisposable commented Mar 18, 2025

Uh oh!

tarekgh commented Mar 18, 2025

Uh oh!

IDisposable commented Mar 18, 2025

Uh oh!

tarekgh commented Mar 23, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

IDisposable commented Mar 14, 2025 •

edited

Loading

IDisposable commented Mar 14, 2025 •

edited

Loading

IDisposable commented Mar 18, 2025 •

edited

Loading

IDisposable Mar 18, 2025 •

edited

Loading