zcsizmadia commented on a change in pull request #1622:
URL: https://github.com/apache/avro/pull/1622#discussion_r834444112



##########
File path: lang/csharp/src/apache/test/Generic/GenericTests.cs
##########
@@ -37,6 +37,45 @@ private static void test<T>(string s, T value)
             Assert.AreEqual(value, output);
         }
 
+        [Test]

Review comment:
       We need more unit tests here. Something like this:
   
   ```
           private static IEnumerable<TestCaseData> 
ConvertsDefaultToLogicalTypeSource = new List<TestCaseData>()
           {
               new TestCaseData(@"{""type"": ""bytes"", ""logicalType"": 
""decimal"", ""precision"": 4}", @"""0""", (decimal)0),
               new TestCaseData(@"{""type"": ""bytes"", ""logicalType"": 
""decimal"", ""precision"": 4}", @"""1234""", (decimal)1234),
               new TestCaseData(@"{""type"": ""bytes"", ""logicalType"": 
""decimal"", ""precision"": 4}", @"""1.234""", (decimal)1.234),
               new TestCaseData(@"{""type"": ""string"", ""logicalType"": 
""uuid""}", @"""00000000-0000-0000-0000-000000000000""", new Guid()),
               new TestCaseData(@"{""type"": ""string"", ""logicalType"": 
""uuid""}", @"""00000000000000000000000000000000""", new Guid()),
               new TestCaseData(@"{""type"": ""string"", ""logicalType"": 
""uuid""}", @"""12345678-1234-5678-1234-123456789012""", new 
Guid("12345678-1234-5678-1234-123456789012")),
               new TestCaseData(@"{""type"": ""string"", ""logicalType"": 
""uuid""}", @"""12345678123456781234123456789012""", new 
Guid("12345678-1234-5678-1234-123456789012")),
               new TestCaseData(@"{""type"": ""int"", ""logicalType"": 
""date""}", "0", DateTime.UnixEpoch),
               new TestCaseData(@"{""type"": ""int"", ""logicalType"": 
""date""}", "123456", DateTime.UnixEpoch.AddDays(123456)),
               new TestCaseData(@"{""type"": ""long"", ""logicalType"": 
""time-micros""}", "0", new TimeSpan()),
               new TestCaseData(@"{""type"": ""long"", ""logicalType"": 
""time-micros""}", "123456", new 
TimeSpan(123456*TimeSpan.TicksPerMillisecond/1000)),
               new TestCaseData(@"{""type"": ""int"", ""logicalType"": 
""time-millis""}", "0", new TimeSpan()),
               new TestCaseData(@"{""type"": ""int"", ""logicalType"": 
""time-millis""}", "123456", new TimeSpan(0, 0, 0, 0, 123456)),
               new TestCaseData(@"{""type"": ""long"", ""logicalType"": 
""timestamp-micros""}", "0", DateTime.UnixEpoch),
               new TestCaseData(@"{""type"": ""long"", ""logicalType"": 
""timestamp-micros""}", "123456", 
DateTime.UnixEpoch.AddTicks(123456*TimeSpan.TicksPerMillisecond/1000)),
               new TestCaseData(@"{""type"": ""long"", ""logicalType"": 
""timestamp-millis""}", "0", DateTime.UnixEpoch),
               new TestCaseData(@"{""type"": ""long"", ""logicalType"": 
""timestamp-millis""}", "123456", DateTime.UnixEpoch.AddMilliseconds(123456))
           };
   
           [TestCaseSource(nameof(ConvertsDefaultToLogicalTypeSource))]
           public void ConvertsDefaultToLogicalType(string typeDefinition, 
string defaultDefinition, object expected)
           {
               var writerSchemaString = @"{
       ""type"": ""record"",
       ""name"": ""Foo"",
       ""fields"": [      
       ]
   }";
   
               var readerSchemaString = $@"{{
       ""type"": ""record"",
       ""name"": ""Foo"",
       ""fields"": [
           {{
               ""name"": ""x"",
               ""type"": {typeDefinition},
               ""default"": {defaultDefinition}
           }}
       ]
   }}";
               var writerSchema = Schema.Parse(writerSchemaString);
   
               Stream stream;
   
               serialize(writerSchemaString,
                   mkRecord(new object[] { }, (RecordSchema) writerSchema),
                   out stream,
                   out _);
   
               var output = deserialize<GenericRecord>(stream, writerSchema, 
Schema.Parse(readerSchemaString));
   
               Assert.AreEqual(expected, output.GetValue(0));
           }
   ```
   
   You will note that some unit tests do not expect the way I think they should 
work, so those cases must be discussed. 
   
   1. microsec logical types swallow/ignore the actual us values and work only 
for milliseconds.
   2. decimal defaults do not work IMO as expected, however I might have been 
using them wrong in the test cases

##########
File path: lang/csharp/src/apache/test/Specific/SpecificTests.cs
##########
@@ -440,6 +441,69 @@ public void TestEmbeddedGenerics()
             Assert.AreEqual(0, dstRecord.UserMatrix[2].Count);
         }
 
+        private static void serializeGeneric<T>(string writerSchema, T actual, 
out Stream stream, out Schema ws)
+        {
+            var ms = new MemoryStream();
+            Encoder e = new BinaryEncoder(ms);
+            ws = Schema.Parse(writerSchema);
+            GenericWriter<T> w = new GenericWriter<T>(ws);
+            w.Write(actual, e);
+            ms.Flush();
+            ms.Position = 0;
+            stream = ms;
+        }
+
+
+        [Test]
+        public void DeserializeToLogicalTypeWithDefault()
+        {
+            var writerSchemaString = @"{
+    ""type"": ""record"",
+    ""name"": ""RecordWithOptionalLogicalType"",
+    ""namespace"": ""Avro.Test.Specific.return"",
+    ""fields"": [      
+    ]}";
+
+            var writerSchema = Schema.Parse(writerSchemaString);
+
+            Stream stream;
+
+            serializeGeneric(writerSchemaString,
+                mkRecord(new object[] { }, (RecordSchema)writerSchema),
+                out stream,
+                out _);
+
+            RecordWithOptionalLogicalType output = 
deserialize<RecordWithOptionalLogicalType>(stream, writerSchema, 
RecordWithOptionalLogicalType._SCHEMA);
+
+            Assert.AreEqual(output.x, new DateTime(1970, 1, 11));
+
+        }
+
+        private static GenericRecord mkRecord(object[] kv, RecordSchema s)

Review comment:
       This seems to be the exact replica of the GenericTests.mkRecord() 
function. Can we make that public and use it, instead uf duplicationg it?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to