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 an effect 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]
