[Issue #44] Update Json parsing to ignore datetime in string (#45)

* [Issue #44] Update Json parsing to ignore datetime in string * Add tests using object snapshots for datetime parsing * Updating changelog for the 2.2.1 release
theramis · Jan 14, 2020 · f5d89cf · f5d89cf
1 parent 8e6df54
commit f5d89cf
Show file tree

Hide file tree

Showing 9 changed files with 117 additions and 11 deletions.
diff --git a/docs/changelog.md b/docs/changelog.md
@@ -6,6 +6,10 @@ nav_order: 99
 # Changelog
 All notable changes to the Snapper project.
 
+## [2.2.1] - 2020-01-14
+### Bug Fix
+- [Issue #44](https://github.com/theramis/Snapper/issues/44) [PR #45](https://github.com/theramis/Snapper/pull/45) Fixed parsing of datetime strings so that they are treated as string by NewtonSoft. Thanks to [@plitwinski](https://github.com/plitwinski) for surfacing the issue.
+
 ## [2.2.0] - 2019-11-01
 ### Added
 - [PR #31](https://github.com/theramis/Snapper/pull/31) `Snapper` now supports the MSTest framework. Thanks to [@tskimmett](https://github.com/tskimmett) for the contribution.
@@ -115,6 +119,7 @@ The first stable release!
 - **Snapper.Json**: Extends Snapper.Core to provide storing snapshots in Json format
 - **Snapper.Json.Xunit**: Extends Snapper.Json and integrates with the XUnit testing framework.
 
+[2.2.1]: https://github.com/theramis/Snapper/compare/2.2.0...2.2.1
 [2.2.0]: https://github.com/theramis/Snapper/compare/2.1.0...2.2.0
 [2.1.0]: https://github.com/theramis/Snapper/compare/2.0.1...2.1.0
 [2.0.1]: https://github.com/theramis/Snapper/compare/2.0.0...2.0.1

diff --git a/docs/snapper/updating-snapshots.md b/docs/snapper/updating-snapshots.md
@@ -18,5 +18,5 @@ Snapper checks for an environment variable called `UpdateSnapshots`. If the valu
 ## Update snapshot attribute
 Snapper will check for a `[UpdateSnapshot]` attribute when running tests. If it finds that a test method/class/assembly has the attribute it will update snapshots for all tests that are run.
 
-By default when this is used attribute, Snapper will try detect whether the tests are running in a CI environment. If a CI environment is detected then the presence of the `[UpdateSnapshots]` will be ignored.
+By default when the attribute is used, Snapper will try detect whether the tests are running in a CI environment. If a CI environment is detected then the presence of the `[UpdateSnapshots]` will be ignored.
 This can be disabled by setting the `ignoreIfCi` flag to false on the attribute. e.g. `[UpdateSnapshots(false)]`
diff --git a/project/Snapper/Json/JObjectHelper.cs b/project/Snapper/Json/JObjectHelper.cs
@@ -0,0 +1,27 @@
+using System.IO;
+using Newtonsoft.Json;
+using Newtonsoft.Json.Linq;
+
+namespace Snapper.Json
+{
+    internal static class JObjectHelper
+    {
+        private static readonly JsonSerializerSettings JsonSettings = new JsonSerializerSettings
+        {
+            DateParseHandling = DateParseHandling.None
+        };
+
+        public static JObject ParseFromString(string jsonString)
+        {
+            return JsonConvert.DeserializeObject(jsonString, JsonSettings) as JObject;
+        }
+
+        public static JObject FromObject(object obj)
+        {
+            if (obj is JObject result)
+                return result;
+
+            return JObject.FromObject(obj, JsonSerializer.Create(JsonSettings));
+        }
+    }
+}
diff --git a/project/Snapper/Json/JsonDiffGenerator.cs b/project/Snapper/Json/JsonDiffGenerator.cs
@@ -3,7 +3,6 @@
 using System.IO;
 using System.Text;
 using DiffMatchPatch;
-using Newtonsoft.Json.Linq;
 
 namespace Snapper.Json
 {
@@ -14,8 +13,8 @@ internal static class JsonDiffGenerator
 
         public static string GetDiffMessage(object currentSnapshot, object newSnapshot)
         {
-            var currentSnapshotJObject = JObject.FromObject(currentSnapshot);
-            var newSnapshotJObject = JObject.FromObject(newSnapshot);
+            var currentSnapshotJObject = JObjectHelper.FromObject(currentSnapshot);
+            var newSnapshotJObject = JObjectHelper.FromObject(newSnapshot);
 
             var dmp = new diff_match_patch();
             var a = dmp.diff_linesToChars(currentSnapshotJObject.ToString(), newSnapshotJObject.ToString());

diff --git a/project/Snapper/Json/JsonSnapshotComparer.cs b/project/Snapper/Json/JsonSnapshotComparer.cs
@@ -7,10 +7,10 @@ internal class JsonSnapshotComparer : ISnapshotComparer
     {
         public bool CompareSnapshots(object oldSnap, object newSnap)
         {
-            var old = JObject.FromObject(oldSnap);
+            var old = JObjectHelper.FromObject(oldSnap);
 
-            var @new = JObject.FromObject(newSnap);
-            @new = JObject.Parse(@new.ToString());
+            var @new = JObjectHelper.FromObject(newSnap);
+            @new = JObjectHelper.ParseFromString(@new.ToString());
             return JToken.DeepEquals(old, @new);
         }
     }

diff --git a/project/Snapper/Json/JsonSnapshotSanitiser.cs b/project/Snapper/Json/JsonSnapshotSanitiser.cs
@@ -1,5 +1,4 @@
 using System;
-using Newtonsoft.Json.Linq;
 using Snapper.Exceptions;
 
 namespace Snapper.Json
@@ -16,7 +15,7 @@ public object SanitiseSnapshot(object snapshot)
                     {
                         try
                         {
-                            return JObject.Parse(stringSnapshot);
+                            return JObjectHelper.ParseFromString(stringSnapshot);
                         }
                         catch (Exception e)
                         {
@@ -25,7 +24,7 @@ public object SanitiseSnapshot(object snapshot)
                     }
                 }
 
-                JObject.FromObject(snapshot);
+                JObjectHelper.FromObject(snapshot);
                 return snapshot;
             }
             catch (ArgumentException)

diff --git a/project/Snapper/Json/JsonSnapshotStore.cs b/project/Snapper/Json/JsonSnapshotStore.cs
@@ -12,7 +12,7 @@ public object GetSnapshot(SnapshotId snapshotId)
             if (!File.Exists(snapshotId.FilePath))
                 return null;
 
-            var fullSnapshot = JObject.Parse(File.ReadAllText(snapshotId.FilePath));
+            var fullSnapshot = JObjectHelper.ParseFromString(File.ReadAllText(snapshotId.FilePath));
 
             if (snapshotId.PrimaryId == null && snapshotId.SecondaryId == null)
                 return fullSnapshot;

diff --git a/project/Tests/Snapper.Tests/SnapperNewtonsoftNuisancesTests.cs b/project/Tests/Snapper.Tests/SnapperNewtonsoftNuisancesTests.cs
@@ -0,0 +1,68 @@
+using System;
+using System.Runtime.CompilerServices;
+using Snapper.Attributes;
+using Xunit;
+
+namespace Snapper.Tests
+{
+    /// <summary>
+    ///     This class contains tests which test that the nuisances of newtonsoft are avoided in Snapper
+    /// </summary>
+    [StoreSnapshotsPerClass]
+    public class SnapperNewtonsoftNuisancesTests
+    {
+        [Fact]
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        // Related issue https://github.com/JamesNK/Newtonsoft.Json/issues/862
+        public void DateTimeIsParsedAsStringBySnapper_UsingStringSnapshot_FileSnapshot()
+        {
+            const string snapshot = "{" +
+                                        "\"Key\" : \"2010-12-31T00:00:00+00:00\"" +
+                                    "}";
+
+            snapshot.ShouldMatchSnapshot();
+        }
+
+        [Fact]
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        // Related issue https://github.com/JamesNK/Newtonsoft.Json/issues/862
+        public void DateTimeIsParsedAsStringBySnapper_UsingObjectSnapshot_FileSnapshot()
+        {
+            var snapshot = new
+            {
+                Key = new DateTime(2010, 12, 31, 0, 0, 0, DateTimeKind.Utc)
+            };
+
+            snapshot.ShouldMatchSnapshot();
+        }
+
+        [Fact]
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        // Related issue https://github.com/JamesNK/Newtonsoft.Json/issues/862
+        public void DateTimeIsParsedAsStringBySnapper_UsingStringSnapshot_InlineSnapshot()
+        {
+            const string snapshot = "{" +
+                                    "\"Key\" : \"2010-12-31T00:00:00+00:00\"" +
+                                    "}";
+
+            snapshot.ShouldMatchInlineSnapshot("{" +
+                                               "\"Key\" : \"2010-12-31T00:00:00+00:00\"" +
+                                               "}");
+        }
+
+        [Fact]
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        // Related issue https://github.com/JamesNK/Newtonsoft.Json/issues/862
+        public void DateTimeIsParsedAsStringBySnapper_UsingObjectSnapshot_InlineSnapshot()
+        {
+            var snapshot = new
+            {
+                Key = new DateTime(2010, 12, 31, 0, 0, 0, DateTimeKind.Utc)
+            };
+
+            snapshot.ShouldMatchInlineSnapshot("{" +
+                                               "\"Key\" : \"2010-12-31T00:00:00Z\"" +
+                                               "}");
+        }
+    }
+}
diff --git a/project/Tests/Snapper.Tests/_snapshots/SnapperNewtonsoftNuisancesTests.json b/project/Tests/Snapper.Tests/_snapshots/SnapperNewtonsoftNuisancesTests.json
@@ -0,0 +1,8 @@
+{
+  "DateTimeIsParsedAsStringBySnapper_UsingStringSnapshot_FileSnapshot": {
+    "Key": "2010-12-31T00:00:00+00:00"
+  },
+  "DateTimeIsParsedAsStringBySnapper_UsingObjectSnapshot_FileSnapshot": {
+    "Key": "2010-12-31T00:00:00Z"
+  }
+}