Conversation
| context 'given a number' do | ||
| let(:number) {1234567890} | ||
| # RUBY 2.5.0 | ||
| # let(:md5) { 'cbabd6f8bfda13b76c0e28eb0a6f4ef1' } |
There was a problem hiding this comment.
Does Ruby 2.5 generate a different hash?
There was a problem hiding this comment.
With Ruby 2.4:
Failed examples:
rspec ./spec/consistency_spec.rb:17 # Crimp.signature check MD5 consistent across versions given a Hash should eq "1dd744d51279187cc08cabe240f98be2"
rspec ./spec/consistency_spec.rb:33 # Crimp.signature check MD5 consistent across versions given an Array should eq "cd29980f258eef3faceca4f4da02ec65"
rspec ./spec/consistency_spec.rb:40 # Crimp.signature check MD5 consistent across versions given legacy Array should eq "4dc4e1161c9315db0bc43c0201b3ec05"
There was a problem hiding this comment.
this probably happens because in crimp.rb we stringify the object with:
def self.to_string(obj)
"#{obj}#{obj.class}"
endin ruby 2.1 an integer class is a Fixnum where in Ruby > 2.4 is a Integer
There was a problem hiding this comment.
Ruby 2.1
2.class.ancestors
[Fixnum, Integer, Numeric, Comparable, Object, PP::ObjectMixin, Kernel, BasicObject]Ruby 2.4.2
2.class.ancestors
[Integer, Numeric, Comparable, Object, PP::ObjectMixin, Kernel, BasicObject]Floats are not showing the same issues, as both 2.1 and 2.4 are returning:
6.6.class.ancestors
[Float, Numeric, Comparable, Object, Kernel, BasicObject]There was a problem hiding this comment.
Wondering if makes sense to have the class part of objects which are not Strings|Hashes|Arrays as this would make very hard to build a new Crimp library in a different language as would require some sort of mapping between object class names...
There was a problem hiding this comment.
I'll create a PR to fix the Fixnum issue so that Crimp can maintain backwards compatibility, but this calls for a review of the gem to make it more language/version agnostic.
There was a problem hiding this comment.
ah also there is still a possible issue with Bignum objects:
Ruby 2.1
100000000000000000000000000000000000000000000000000000.class
=> BignumRuby >= 2.4
100000000000000000000000000000000000000000000000000000.class
=> IntegerBut for now I'd try to ignore and review it with the new version
3 participants
JIRA: https://jira.dev.bbc.co.uk/browse/DATAPRES-417
DO NOT MERGE!
Development branch/PR to compare MD5/crimping output with a WIP Node implementation. Usefulness of the code to be ascertained near the end of development; initial test checks for MD5 hash outputted from a stringified Ruby hash (which the Node implementation might have to be dealing with).