diff --git a/lib/super_diff/basic/operation_tree_builders/hash.rb b/lib/super_diff/basic/operation_tree_builders/hash.rb index ba030524..d07e9553 100644 --- a/lib/super_diff/basic/operation_tree_builders/hash.rb +++ b/lib/super_diff/basic/operation_tree_builders/hash.rb @@ -118,18 +118,7 @@ def unary_operations_using_variant_of_patience_algorithm # match in 'actual', then stop, because it's going to be # handled in some future iteration of the 'actual' loop. break - elsif aks[ai + 1..].any? do |k| # rubocop:disable Lint/DuplicateBranch for clarity - expected.include?(k) && expected[k] != actual[k] - end - - # While we backtracked a bit to iterate over 'expected', we - # now have to look ahead. If we will end up encountering a - # insert that matches this delete later, stop and go back to - # iterating over 'actual'. This is because the delete we would - # have added now will be added later when we encounter the - # associated insert, so we don't want to add it twice. - break - else + elsif operations.none? { |op| op.key == ek } operations << Core::UnaryOperation.new( name: :delete, collection: expected, diff --git a/spec/unit/basic/operation_tree_builders/hash_spec.rb b/spec/unit/basic/operation_tree_builders/hash_spec.rb new file mode 100644 index 00000000..008a6825 --- /dev/null +++ b/spec/unit/basic/operation_tree_builders/hash_spec.rb @@ -0,0 +1,122 @@ +# frozen_string_literal: true + +require 'securerandom' +require 'spec_helper' + +RSpec.describe SuperDiff::Basic::OperationTreeBuilders::Hash do + describe 'the produced tree' do + subject(:tree) { described_class.call(expected: expected, actual: actual) } + + let(:actual) do + { + 'A' => :c, + 'B' => :x, + 'C' => :b + } + end + + let(:expected) do + { + 'C' => :aaa, + 'B' => :x, + 'A' => :zzz + } + end + + it 'has at most two entries per key' do + groups = tree.group_by(&:key) + + groups.each do |key, value| + expect(value.count).to(be <= 2, "There are > 2 operations for key `#{key}`") + end + end + + context 'with large random hashes' do + let(:operation_tree) { described_class.call(expected: expected, actual: actual) } + let(:actual) do + Array.new(rand(1..50)) do |i| + [i, SecureRandom.alphanumeric(4)] + end.to_h + end + let(:expected) do + entries = actual.entries + + # Remove keys at random + entries = entries.sample(rand(actual.length)) + + # Change keys at random + entries = entries.map do |k, v| + [k, rand(5).zero? ? SecureRandom.alphanumeric(4) : v] + end + + # Add keys at random + max_key = actual.keys.max || 0 + entries += Array.new(rand(10)) do |i| + [max_key + i + 1, SecureRandom.alphanumeric(4)] + end + + entries.shuffle.to_h + end + + it 'is correct' do + (actual.keys | expected.keys).each do |key| + relevant_operations = operation_tree.select { |op| op.key == key } + + if actual.key?(key) + if expected.key?(key) + if actual[key] == expected[key] + # no-op + expect(relevant_operations).to contain_exactly( + having_attributes(name: :noop) + ) + else + # remove, followed by add + expect(relevant_operations).to contain_exactly( + having_attributes(name: :delete), + having_attributes(name: :insert) + ) + end + else + # in actual but not expected, there should be one addition + expect(relevant_operations).to contain_exactly( + having_attributes(name: :insert) + ) + end + else + # in expected but not actual; there should be one removal + expect(relevant_operations).to contain_exactly( + having_attributes(name: :delete) + ) + end + end + end + end + + context 'with expected keys interleaved with actual' do + let(:operation_tree) { described_class.call(expected: expected, actual: actual) } + + let(:actual) do + { + 'A' => 1, + 'C' => 3 + } + end + let(:expected) do + { + 'A' => 1, + 'B' => 2, + 'C' => 3 + } + end + + it 'interleaves the delete operation' do + operations = operation_tree.to_a + expect(operations.count).to eq(3) + expect(operations[1]).to have_attributes( + key: 'B', + name: :delete + ) + end + end + end +end