Add DateTime ParseExact test with 'Allow White' styles#27678
Add DateTime ParseExact test with 'Allow White' styles#27678ahsonkhan merged 1 commit intodotnet:masterfrom
Conversation
|
|
||
|
|
||
| yield return new object[] { "9", "\" \"%d", CultureInfo.InvariantCulture, DateTimeStyles.AllowLeadingWhite, new DateTime(DateTime.Now.Year, 1, 9, 0, 0, 0) }; | ||
| yield return new object[] { "15", "\' \'dd", CultureInfo.InvariantCulture, DateTimeStyles.AllowLeadingWhite, new DateTime(DateTime.Now.Year, 1, 15, 0, 0, 0) }; |
There was a problem hiding this comment.
Maybe I'm misunderstanding, but doesn't AllowLeadingWhitespace primarily apply to ignoring whitespace in the input string, not the pattern?
There was a problem hiding this comment.
That isn't what the implementation suggests. Quoted white space within the format string is allowed. Although, I couldn't really find good documentation on it to show it is supported. MSDN only talks about the input string.
There was a problem hiding this comment.
https://msdn.microsoft.com/en-us/library/ms131038(v=vs.110).aspx
AllowLeadingWhite - Specifies that white space not defined by format can appear at the beginning of s.
https://referencesource.microsoft.com/#mscorlib/system/globalization/datetimeparse.cs,4038
There was a problem hiding this comment.
Specifies that white space not defined by format can appear at the beginning of s.
Right... that's not what your tests are testing... the input strings you added have no whitespace... what am I missing?
There was a problem hiding this comment.
You wanted me to add tests to cover all the code paths in RemoveLeadingInQuoteSpaces() / RemoveTrailingInQuoteSpaces(). Those methods are only ever called when removing white space from the format string (not from the input string). The tests I added improve branch coverage of those two internal methods.
I understand that the tests I added do not verify the correctness of the whitespace in the input string (and we should definitely add those too).
If there is a bug in the implementation of TryParseExact, we should address that, but I am not sure what the expected behavior should be based on the docs.
There was a problem hiding this comment.
Those methods are only ever called when removing white space from the format string (not from the input string).
Ah, that's what I was missing, thanks.
…#27678) Commit migrated from dotnet/corefx@d907a94
3 participants
See dotnet/coreclr#16718 (comment) for context.
cc @jkotas, @stephentoub
Added some test to cover code paths leading to RemoveLeadingInQuoteSpaces / RemoveTrailingInQuoteSpaces. There are other DateTimeStyles that are still not well covered. Filed issue - https://github.com/dotnet/corefx/issues/27679