zcsizmadia commented on a change in pull request #1622:
URL: https://github.com/apache/avro/pull/1622#discussion_r835377983
##########
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:
How about this. Leave the tests as they are. I feel that we have
identiifed a missing code coverage, which indirectly has on your unit tests. As
much as I dislike to piggyback additional changes to PR which are not related,
this is kinda connected in some sense. Here is the unit tests to validate the
conversions for Decimal and could be added to `LogicalTypeTests.cs`. I think
the comments will make it trivial when the code is reviewed, so a different PR
might not be needed. If you feel you rather have in in a different PR, that is
ok and I will do that and we will sequence merging the PRs properly:
```
[TestCase("0", 0, new byte[] { 0 })]
[TestCase("000000000000000001.01", 2, new byte[] { 101 })]
[TestCase("123456789123456789.56", 2, new byte[] { 0, 171, 84, 169,
143, 129, 101, 36, 108 })]
[TestCase("1234", 0, new byte[] { 4, 210 })]
[TestCase("1234.5", 1, new byte[] { 48, 57 })]
[TestCase("1234.56", 2, new byte[] { 1, 226, 64 })]
[TestCase("-0", 0, new byte[] { 0 })]
[TestCase("-000000000000000001.01", 2, new byte[] { 155 })]
[TestCase("-123456789123456789.56", 2, new byte[] { 255, 84, 171,
86, 112, 126, 154, 219, 148 })]
[TestCase("-1234", 0, new byte[] { 251, 46 })]
[TestCase("-1234.5", 1, new byte[] { 207, 199 })]
[TestCase("-1234.56", 2, new byte[] { 254, 29, 192 })]
// This tests ensures that changes to Decimal.ConvertToBaseValue and
ConvertToLogicalValue can be validated (bytes)
public void TestDecimalConvert(string s, int scale, byte[] converted)
{
var schema = (LogicalSchema)Schema.Parse(@$"{{""type"":
""bytes"", ""logicalType"": ""decimal"", ""precision"": 4, ""scale"":
{scale}}}");
var avroDecimal = new Avro.Util.Decimal();
var decimalVal = (AvroDecimal)decimal.Parse(s);
// TestDecimal tests
ConvertToLogicalValue(ConvertToBaseValue(...)) which might hide symmetrical
breaking changes in both functions
// The following 2 tests are checking the conversions seperately
// Validate Decimal.ConvertToBaseValue
Assert.AreEqual(converted,
avroDecimal.ConvertToBaseValue(decimalVal, schema));
// Validate Decimal.ConvertToLogicalValue
Assert.AreEqual(decimalVal,
(AvroDecimal)avroDecimal.ConvertToLogicalValue(converted, schema));
}
```
--
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]
