Azure / autorest

OpenAPI (f.k.a Swagger) Specification code generator. Supports C#, PowerShell, Go, Java, Node.js, TypeScript, Python
MIT License
4.62k stars 739 forks source link

Some DateTime serialization unit tests are timezone relative #296

Closed codespare closed 8 years ago

codespare commented 9 years ago

Running unit tests locally in AEST time zone, DateTime serialization tests end up with results similar to:

autorest\master\ClientRuntimes\CSharp\ClientRuntime.Tests\JsonSerializationTests.cs(250): error : Microsoft.Rest.ClientRuntime.Tests.JsonSerializationTests.DateSerializationWithMaxValue [FAIL] [\autorest\master\build.proj]
              Assert.Equal() Failure
                                            ↓ (pos 45)
              Expected:   "dt": "9999-12-31T23:59:59Z",\r\n  "dn": "9999-12-31T23:59:59
              Actual:     "dt": "9999-12-31T12:59:59.9999999Z",\r\n  "dn": "9999-12-31T
                                            ↑ (pos 45)

As you specify a timezone in the test strings, I think you should use DateTimeStyles.AssumeUniversal instead of AdjustToUniversal, otherwise outcome depends on host timezone as there is a conversion to local time.

markcowl commented 9 years ago

@codespare Thanks for the report we will take a look

devigned commented 9 years ago

Can you try executing these against the dev branch. I think we have solved this as the tests are executing in Travis? I believe has the machine configured to UTC and thus caused these issues to corrected.

devigned commented 9 years ago

@codespare if you find this works for you, please ping back.

codespare commented 9 years ago

Off dev commit 06384a26b4381f2b21056c044a4770cfaaeb5f81, running default gulp, I get the following 3 failures still:

xUnit.net Console Runner (32-bit .NET 4.0.30319.42000)
  Discovering: ClientRuntime.Tests
  Discovered:  ClientRuntime.Tests
  Starting:    ClientRuntime.Tests
    Microsoft.Rest.ClientRuntime.Tests.JsonSerializationTests.DateSerializationWithMaxValue [FAIL]
      Assert.Equal() Failure
                                       ↓ (pos 45)
    Microsoft.Rest.ClientRuntime.Tests.JsonSerializationTests.DateSerializationWithoutNulls [FAIL]
      Expected: ���  "dt": "9999-12-31T23:59:59Z",\r\n  "dn": "9999-12-31T23:59:59���
      Actual:   ���  "dt": "9999-12-31T12:59:59.9999999Z",\r\n  "dn": "9999-12-31T���
                                       ↑ (pos 45)
      Stack Trace:
        ClientRuntimes\CSharp\ClientRuntime.Tests\JsonSerializationTests.cs(249,0): at Microsoft.Rest.ClientRuntime.Tests.JsonSerializationT
ests.DateSerializationWithMaxValue()
      Assert.Equal() Failure
                                      ↓ (pos 20)
      Expected: {\r\n  "d": "2015-06-01",\r\n  "dt": "2015-06-01T23:10:08.121Z",\r���
      Actual:   {\r\n  "d": "2015-06-02",\r\n  "dt": "2015-06-01T23:10:08.121Z",\r���
                                      ↑ (pos 20)
      Stack Trace:
        ClientRuntimes\CSharp\ClientRuntime.Tests\JsonSerializationTests.cs(183,0): at Microsoft.Rest.ClientRuntime.Tests.JsonSerializationT
ests.DateSerializationWithoutNulls()
    Microsoft.Rest.ClientRuntime.Tests.JsonSerializationTests.DateSerializationWithNulls [FAIL]
      System.FormatException : The UTC representation of the date falls outside the year range 1-9999.
      Stack Trace:
           at System.DateTimeParse.ParseExact(String s, String format, DateTimeFormatInfo dtfi, DateTimeStyles style, TimeSpan& offset)
           at System.DateTimeOffset.ParseExact(String input, String format, IFormatProvider formatProvider, DateTimeStyles styles)
           at Newtonsoft.Json.Converters.IsoDateTimeConverter.ReadJson(JsonReader reader, Type objectType, Object existingValue, JsonSeriali
zer serializer)
           at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.DeserializeConvertable(JsonConverter converter, JsonReader reader,
Type objectType, Object existingValue)
           at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.SetPropertyValue(JsonProperty property, JsonConverter propertyConve
rter, JsonContainerContract containerContract, JsonProperty containerProperty, JsonReader reader, Object target)
           at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.PopulateObject(Object newObject, JsonReader reader, JsonObjectContr
act contract, JsonProperty member, String id)
           at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateObject(JsonReader reader, Type objectType, JsonContract contr
act, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
           at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateValueInternal(JsonReader reader, Type objectType, JsonContrac
t contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
           at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.Deserialize(JsonReader reader, Type objectType, Boolean checkAdditi
onalContent)
           at Newtonsoft.Json.JsonSerializer.DeserializeInternal(JsonReader reader, Type objectType)
           at Newtonsoft.Json.JsonConvert.DeserializeObject(String value, Type type, JsonSerializerSettings settings)
           at Newtonsoft.Json.JsonConvert.DeserializeObject[T](String value, JsonSerializerSettings settings)
        ClientRuntimes\CSharp\ClientRuntime.Tests\JsonSerializationTests.cs(209,0): at Microsoft.Rest.ClientRuntime.Tests.JsonSerializationT
ests.DateSerializationWithNulls()
  Finished:    ClientRuntime.Tests
=== TEST EXECUTION SUMMARY ===
   ClientRuntime.Tests  Total: 54, Errors: 0, Failed: 3, Skipped: 0, Time: 1.167s
codespare commented 9 years ago

Looking into it in more details, those modifications pass the tests:

DateSerializationWithMaxValue with

""dton"": """ + new DateTimeOffset(localDateTime).ToString("yyyy-MM-ddTHH:mm:sszzz") + @"""

because 'zzz' is local time zone relative for DateTime (but not DateTimeOffset).

DateSerializationWithoutNulls with

test.Date = localDateTimeOffset.UtcDateTime;
test.DateNullable = localDateTimeOffset.UtcDateTime;
test.DateTime = localDateTimeOffset.UtcDateTime;
test.DateTimeNullable = localDateTimeOffset.UtcDateTime;

as I think it is what the test tries to show.

I can't find a fix to DateSerializationWithNulls without significant changes though.

fearthecowboy commented 8 years ago

Howdy!

In our planning for driving towards a stable '1.0' release, I'm marking this issue as 'deferred' :zzz: and we're going to review it during the post-1.0 planning cycle.

It's not to say that we're not going to work on it, or that this isn't not important, but at the moment, we're picking and choosing the stuff we must do before 1.0. :horse_racing: :horse_racing: :horse_racing:

We'll make sure we pick this back up at that point. :tada: