zcsizmadia commented on a change in pull request #1622:
URL: https://github.com/apache/avro/pull/1622#discussion_r835213902
##########
File path: lang/csharp/src/apache/test/Generic/GenericTests.cs
##########
@@ -17,28 +17,64 @@
*/
using System;
using System.IO;
-using System.Linq;
using Avro.IO;
using System.Collections.Generic;
+using System.Text;
using Avro.Generic;
using NUnit.Framework;
+using Decoder = Avro.IO.Decoder;
+using Encoder = Avro.IO.Encoder;
namespace Avro.Test.Generic
{
class GenericTests
{
- private static void test<T>(string s, T value)
+ private static string intToUtf8(int value)
{
- Stream ms;
- Schema ws;
- serialize(s, value, out ms, out ws);
- Schema rs = Schema.Parse(s);
- T output = deserialize<T>(ms, ws, rs);
- Assert.AreEqual(value, output);
+ var decimalLogicalType = new Avro.Util.Decimal();
+ var logicalSchema = (LogicalSchema)
+ Schema.Parse(@"{ ""type"": ""bytes"", ""logicalType"":
""decimal"", ""precision"": 4 }");
+
+ byte[] byteArray =
(byte[])decimalLogicalType.ConvertToBaseValue(new AvroDecimal(value),
logicalSchema);
Review comment:
I have some concern converting the int value to the decimal
implementation in the unit test code. I am worried that we might hide some
issues in the unit tests if e.g. we modify Decimal.ConvertToBaseValue with
some major breaking change, however the unit test will pass, but the new
format is not compatible with the previous one.
Maybe instead of the intToUtf8(...) function you could hard code the actual
string representation of the decimal default .
something like "\0" for 0m and "\u0004\u00d2" for 1234m. In this case the
unit tests are more rigid and are able to detect changes in the underlying
implementation. This might even help with the TestCaseDource stack overflow,
however , that might not be the case.
--
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]
