[GitHub] [tinkerpop] spmallette commented on a change in pull request #1280: TINKERPOP-2360 gremlin-python: fix up serializer out of range of int

[GitBox <gi...@apache.org>]

spmallette commented on a change in pull request #1280:
URL: https://github.com/apache/tinkerpop/pull/1280#discussion_r415746197



##########
File path: gremlin-python/src/main/jython/tests/structure/io/test_graphsonV2d0.py
##########
@@ -307,6 +307,7 @@ def test_numbers(self):
         assert {"@type": "gx:Byte", "@value": 1} == json.loads(self.graphson_writer.writeObject(int.__new__(SingleByte, 1)))
         assert {"@type": "g:Int64", "@value": 2} == json.loads(self.graphson_writer.writeObject(long(2)))
         assert {"@type": "g:Int32", "@value": 1} == json.loads(self.graphson_writer.writeObject(1))
+        assert {"@type": "g:Int64", "@value": 851401972585122} == json.loads(self.graphson_writer.writeObject(851401972585122))

Review comment:
       Could we please take this opportunity to improve our unit testing in general:
   
   1. Add tests for the negative ranges in the int/long/bigint serializers.
   2. We validate `long(2)` which is in the range of int and coerces to `Int64` but could you please add a test for something like `long(851401972585122)` which is in the range of long to cover that 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org