From ebcd5526feb6efbef78be8e7051244f1402fb346 Mon Sep 17 00:00:00 2001 From: Wayne Conrad Date: Fri, 16 Jun 2017 16:11:21 -0700 Subject: [PATCH 1/2] Correct indentation --- spec/extension/enumerable_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/extension/enumerable_spec.rb b/spec/extension/enumerable_spec.rb index 767bd1b..80e4093 100644 --- a/spec/extension/enumerable_spec.rb +++ b/spec/extension/enumerable_spec.rb @@ -1,7 +1,8 @@ require 'spec_helper' describe Enumerable do - KeyValue = Struct.new(:key, :value) do + + KeyValue = Struct.new(:key, :value) do def <=> (other) self.key <=> other.key end From 18cc08695732835dde19b5d2b6dd61cc28681bd5 Mon Sep 17 00:00:00 2001 From: Wayne Conrad Date: Fri, 16 Jun 2017 15:34:33 -0700 Subject: [PATCH 2/2] Make tests more reliable, especially for MRI >= 2.2 * Monkey-patch built-sorts to force them to be unstable in all Rubies. Some rubies (such as MRI >= 2.2) have stable sorts, which can cause the tests to pass erroneously. See https://stackoverflow.com/a/44486562/238886 for more about sort stability in Ruby * Set the random number seed in order to make tests repeatable. Since the tests sort relatively short arrays, random chance would sometimes cause erroneous test failures. * Some tests checked that Ruby's built-in sort is unstable. Those checks were moved into their own sections and expanded. Now every Ruby built-in sort method is tested for to ensure that it is unstable. --- spec/extension/array_spec.rb | 78 +++++++++++++++++++++++ spec/extension/enumerable_spec.rb | 49 ++++++++++++-- spec/spec_helper.rb | 5 ++ spec/support/force_unstable.rb | 102 ++++++++++++++++++++++++++++++ 4 files changed, 229 insertions(+), 5 deletions(-) create mode 100644 spec/support/force_unstable.rb diff --git a/spec/extension/array_spec.rb b/spec/extension/array_spec.rb index 180578f..fdb3fec 100644 --- a/spec/extension/array_spec.rb +++ b/spec/extension/array_spec.rb @@ -24,5 +24,83 @@ def <=> (other) expect(array.to_s).to eq expectation.to_s end end + + # Ensure that Array's built-in sorts are unstable. In some Ruby + # implementations, sorts are stable. These tests force the built-in + # sorts to be unstable, which we ensure here. + describe "built-in sort" do + + describe "Array#sort" do + it "is unstable" do + sorted = array.sort + expect(sorted.to_s).not_to eq expectation.to_s + end + end + + describe "Array#sort with block" do + it "is unstable" do + sorted = array.sort do |a, b| + a.length <=> b.length + end + expect(sorted.to_s).not_to eq expectation.to_s + end + end + + describe "Array#sort!" do + it "is unstable" do + sorted = array.dup + sorted.sort! + expect(sorted.to_s).not_to eq expectation.to_s + end + end + + describe "Array#sort! with block" do + it "is unstable" do + sorted = array.dup + sorted.sort! do |a, b| + a.length <=> b.length + end + expect(sorted.to_s).not_to eq expectation.to_s + end + end + + describe "Array#sort_by" do + it "is unstable" do + unsorted = ['a', 'c', 'bd', 'fe', 'b'] + enum = unsorted.sort_by + sorted = enum.with_index { |e, _i| e.length } + expect(enum).to be_an(Enumerator) + expect(sorted.to_a).not_to eq ['a', 'c', 'b', 'bd', 'fe'] + end + end + + describe "Array#sort_by with block" do + it "is unstable" do + unsorted = ['a', 'c', 'bd', 'fe', 'b'] + sorted = unsorted.sort_by { |e| e.length } + expect(sorted).not_to eq ['a', 'c', 'b', 'bd', 'fe'] + end + end + + describe "Array#sort_by!" do + it "is unstable" do + sorted = ['a', 'c', 'bd', 'fe', 'b'].dup + enum = sorted.sort_by! + enum.with_index { |e, _i| e.length } + expect(enum).to be_an(Enumerator) + expect(sorted.to_a).not_to eq ['a', 'c', 'b', 'bd', 'fe'] + end + end + + describe "Array#sort_by! with block" do + it "is unstable" do + sorted = ['a', 'c', 'bd', 'fe', 'b'].dup + sorted.sort_by! { |e| e.length } + expect(sorted).not_to eq ['a', 'c', 'b', 'bd', 'fe'] + end + end + + end + end diff --git a/spec/extension/enumerable_spec.rb b/spec/extension/enumerable_spec.rb index 80e4093..599656c 100644 --- a/spec/extension/enumerable_spec.rb +++ b/spec/extension/enumerable_spec.rb @@ -13,7 +13,6 @@ def <=> (other) describe "#stable_sort_by (Array)" do it 'sorts stably' do - expect(['a', 'c', 'bd', 'fe', 'b'].sort_by { |x| x.length }).not_to eq ['a', 'c', 'b', 'bd', 'fe'] expect(['a', 'c', 'bd', 'fe', 'b'].stable_sort_by { |x| x.length }).to eq ['a', 'c', 'b', 'bd', 'fe'] expect(['a', 'c', 'bd', 'fe', 'b'].stable_sort_by.is_a?(Enumerator)).to be true end @@ -21,7 +20,6 @@ def <=> (other) describe "#stable_sort_by (Enumerator)" do it 'sorts stably' do - expect(['a', 'c', 'bd', 'fe', 'b'].each.sort_by { |x| x.length }).to eq ['a', 'c', 'b', 'fe', 'bd'] expect(['a', 'c', 'bd', 'fe', 'b'].each.stable_sort_by { |x| x.length }).to eq ['a', 'c', 'b', 'bd', 'fe'] expect(['a', 'c', 'bd', 'fe', 'b'].each.stable_sort_by.is_a?(Enumerator)).to be true end @@ -29,16 +27,57 @@ def <=> (other) describe "#stable_sort (Array)" do it 'sorts stably' do - expect(array.sort.to_s).not_to eq expectation.to_s expect(array.stable_sort.to_s).to eq expectation.to_s end end describe "#stable_sort (Enumerator)" do it 'sorts stably' do - expect(array.each.sort.to_s).not_to eq expectation.to_s expect(array.each.stable_sort.to_s).to eq expectation.to_s end end -end + # Ensure that Enumerable's built-in sorts are unstable. In some + # Ruby implementations, sorts are stable. These tests force the + # built-in sorts to be unstable, which we ensure here. + describe "built-in sort" do + + describe "Enumerable#sort_by" do + it "is unstable" do + unsorted = ['a', 'c', 'bd', 'fe', 'b'].each + enum = unsorted.sort_by + sorted = enum.with_index { |e, _i| e.length } + expect(enum).to be_an(Enumerator) + expect(sorted.to_a).not_to eq ['a', 'c', 'b', 'bd', 'fe'] + end + end + + describe "Enumerable#sort_by with block" do + it "is unstable" do + unsorted = ['a', 'c', 'bd', 'fe', 'b'].each + sorted = unsorted.sort_by { |e| e.length } + expect(sorted).not_to eq ['a', 'c', 'b', 'bd', 'fe'] + end + end + + describe "Enumerable#sort" do + it "is unstable" do + unsorted = array.each + sorted = unsorted.sort + expect(sorted.to_s).not_to eq expectation.to_s + end + end + + describe "Enumerable#sort with block" do + it "is unstable" do + unsorted = array.each + sorted = unsorted.sort do |a, b| + a.length <=> b.length + end + expect(sorted.to_s).not_to eq expectation.to_s + end + end + + end + +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 1836978..fde6416 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -3,3 +3,8 @@ $LOAD_PATH.unshift File.expand_path('../../lib', __FILE__) require 'stable_sort' + +glob = File.join(File.dirname(__FILE__), "support/**/*.rb") +Dir[glob].sort.each do |path| + require path +end diff --git a/spec/support/force_unstable.rb b/spec/support/force_unstable.rb new file mode 100644 index 0000000..5088eb7 --- /dev/null +++ b/spec/support/force_unstable.rb @@ -0,0 +1,102 @@ +# Monkey-patch the default sorts to ensure that they are unstable. We +# do that because sort is stable in some Ruby implementations +# (e.g. MRI >= 2.2 on Linux). Tests run under those implementations +# could pass even if this library were broken. +# +# We monkey-patch all sort methods: Even though in MRI some sort +# methods use other sort methods (for example, Enumerable#sort uses +# Array#sort), some of those bindings are static, and others dynamic. +# A minimal monkey-patch would bind the tests tightly to the Ruby +# implementation, making them fragile. Patching all sort methods +# makes the test more resiliant when the implementation changes. + +RSpec.configure do |c| + c.before do + # To make tests reproducible, set the random number seed before + # each test. Since the tests depend upon sorting relatively short + # lists, and noticing whether the sorted lists are stably sorted + # or not, occasional false test failures result. There's nothing + # special about this seed: it's just one that caused all the tests + # to be reproducible. + # + # This is a bit of a hack, dependent upon the random number + # generator in a given implementation. + # + # An alternative to this is to make the test arrays large enough + # that false negatives are unlikely enough to not be a problem. + # That would be less of a hack, but a larger change to the + # existing tests. + Random.srand(10) + end +end + +class Array + + original_sort_by = instance_method(:sort_by) + define_method(:sort_by) do |*args, &block| + return to_enum(:sort_by) unless block + original_sort_by.bind(self).call do |o| + [block.call(o), rand] + end + end + + original_sort_by_bang = instance_method(:sort_by!) + define_method(:sort_by!) do |*args, &block| + return to_enum(:sort_by!) unless block + original_sort_by_bang.bind(self).call do |o| + [block.call(o), rand] + end + end + + original_sort = instance_method(:sort) + define_method(:sort) do |*args, &block| + if block_given? + original_sort.bind(self).call do |a, b| + [yield(a, b), rand] + end + else + original_sort.bind(self).call do |a, b| + [a, rand] <=> [b, rand] + end + end + end + + original_sort_bang = instance_method(:sort!) + define_method(:sort!) do + if block_given? + original_sort_bang.bind(self).call do |a, b| + [yield(a, b), rand] + end + else + original_sort_bang.bind(self).call do |a, b| + [a, rand] <=> [b, rand] + end + end + end + +end + +module Enumerable + + original_sort = instance_method(:sort) + define_method(:sort) do + if block_given? + original_sort.bind(self).call do |a, b| + [yield(a, b), rand] + end + else + original_sort.bind(self).call do |a, b| + [a, rand] <=> [b, rand] + end + end + end + + original_sort_by = instance_method(:sort_by) + define_method(:sort_by) do |&block| + return to_enum(:sort_by) unless block + original_sort_by.bind(self).call do |o| + [block.call(o), rand] + end + end + +end