Skip to content
This repository has been archived by the owner on Nov 7, 2018. It is now read-only.

DateTime ConvertFromInvariantString test fail ( as expected ) #98

Closed
devel0 opened this issue Nov 6, 2015 · 6 comments
Closed

DateTime ConvertFromInvariantString test fail ( as expected ) #98

devel0 opened this issue Nov 6, 2015 · 6 comments

Comments

@devel0
Copy link

devel0 commented Nov 6, 2015

Running unit tests I got fail at Configure_GetsNullableOptionsFromConfiguration test for given test datetime, this is expected cause the test try to ConvertFromInvariantString at ConfigurationBinder.cs (ReadValue) where passed input string is generated by the OptionsTest.cs(Configure_GetsNullableOptionsFromConfiguration_Data) where a datetime string is generated using current culture instead of the invariant one.
{ nameof(NullableOptions.MyNullableDateTime), new DateTime(1995, 12, 31).ToString(CultureInfo.CurrentCulture.DateTimeFormat.ShortDatePattern) }

@divega divega added this to the 1.0.0-rc2 milestone Nov 6, 2015
@HaoK
Copy link
Member

HaoK commented Dec 4, 2015

@divega what behavior do we want here? Indeed we are always converting from Invariant in the binder, should we try both with Invariant first and then a normal ConvertFromString if the first fails?

@MichaCo
Copy link

MichaCo commented Dec 4, 2015

I'd say it is not a good idea to parse with the current culture. Instead, this test should be fixed by setting the culture correctly.

What would happen if you have a date string formatted with the de-DE culture in your configuration and deploy that to production where the culture is en-US? Yup it would fail...
runs on my machine issue 😿

@HaoK
Copy link
Member

HaoK commented Dec 4, 2015

Yeah I guess what we have is fine, its just a test issue, and invariant string format is the only supported one

@HaoK
Copy link
Member

HaoK commented Dec 4, 2015

@divega is it expected that we need to support running our tests outside of en-US? Either way the priority of this should be low as its not causing any issues unless you run tests with a different culture... Moving to backlog

@HaoK HaoK modified the milestones: Backlog, 1.0.0-rc2 Dec 4, 2015
@HaoK HaoK removed their assignment Dec 4, 2015
@HaoK
Copy link
Member

HaoK commented Dec 9, 2015

758e11e

@HaoK HaoK closed this as completed Dec 9, 2015
@HaoK HaoK modified the milestones: 1.0.0-rc2, Backlog Dec 9, 2015
@HaoK HaoK added the 3 - Done label Dec 9, 2015
@divega
Copy link

divega commented Dec 9, 2015

Also, I have filed aspnet/Configuration#348 to follow up on what behavior we want on the product re culture.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants