Skip to content

Expanded Ruby tests #8

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
coderkind wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Expanded Ruby tests #8

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
def2bdd
Add test for stringified hash
coderkind Aug 7, 2018
ff2542f
Add stringified hash MD5 test
coderkind Aug 7, 2018
ac5878e
Add MD5 string test
coderkind Aug 7, 2018
afc43ed
Remove file added in error
coderkind Aug 7, 2018
b597592
Remove file added in error
coderkind Aug 7, 2018
b0976d6
Add number-based test
coderkind Aug 7, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions spec/consistency_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,29 @@

specify { expect(subject.signature(array)).to eq md5 }
end

context 'given stringified hash' do
let(:stringified_hash) {"{ a: { b: 'b', c: 'c' }, d: 'd' }"}
let(:md5) { 'f5327912eeb41996b5aded6550d11187' }

specify { expect(subject.signature(stringified_hash)).to eq md5 }
end

context 'given a string' do
let(:string) {"do shash'owania"}
let(:md5) { '48e380a165c417253512a904d6b5cf2b' }

specify { expect(subject.signature(string)).to eq md5 }
end

context 'given a number' do
let(:number) {1234567890}
# RUBY 2.5.0
# let(:md5) { 'cbabd6f8bfda13b76c0e28eb0a6f4ef1' }
Copy link
Contributor

@samfrench samfrench Aug 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does Ruby 2.5 generate a different hash?

Copy link
Contributor

@samfrench samfrench Aug 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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"

Copy link
Contributor

@ettomatic ettomatic Aug 8, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this probably happens because in crimp.rb we stringify the object with:

 def self.to_string(obj)
    "#{obj}#{obj.class}"
  end

in ruby 2.1 an integer class is a Fixnum where in Ruby > 2.4 is a Integer

Copy link
Contributor

@ettomatic ettomatic Aug 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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]

Copy link
Contributor

@ettomatic ettomatic Aug 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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...

Copy link
Contributor

@ettomatic ettomatic Aug 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@ettomatic ettomatic Aug 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah also there is still a possible issue with Bignum objects:

Ruby 2.1

100000000000000000000000000000000000000000000000000000.class
=> Bignum

Ruby >= 2.4

100000000000000000000000000000000000000000000000000000.class
=> Integer

But for now I'd try to ignore and review it with the new version

let(:md5) { '1f5c62f597aefd84d180c26e1de61021' }

specify { expect(subject.signature(number)).to eq md5 }
end
end
end
end
10 changes: 10 additions & 0 deletions spec/crimp_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
describe Crimp do
let (:hash) { { a: { b: 'b', c: 'c' }, d: 'd' } }
let (:hash_unordered) { { d: 'd', a: { c: 'c', b: 'b' } } }
let (:stringified_hash) {"{ a: { b: 'b', c: 'c' }, d: 'd' }"}
let (:array) { [1, 2, 3, [4, [5, 6]]] }
let (:array_unordered) { [3, 2, 1, [[5, 6], 4]] }

Expand All @@ -24,6 +25,15 @@
end
end

context 'given a stringified hash' do
specify { expect(subject.signature(stringified_hash)).to be_a String }

it 'returns MD5 hash of stringified hash' do
# puts subject.signature(stringified_hash)
expect(subject.signature(stringified_hash)).to eq(Digest::MD5.hexdigest(subject.stringify(stringified_hash)))
end
end

context 'given an Array' do
specify { expect(subject.signature(array)).to be_a String }

Expand Down