Better newline handling when performing regexp diffs/ignores

Author: kpaulisse

lib/octocatalog-diff/catalog-diff/differ.rb

@@ -294,10 +294,13 @@ def regexp_operator_match?(operator, regex, old_val, new_val)
         # Use diffy to get only the lines that have changed in a text object.
         # As we iterate through the diff, jump out if we have our answer: either
         # true if '=~>' finds ANY match, or false if '=&>' fails to find a match.
-        Diffy::Diff.new(old_val, new_val, context: 0).each do |line|
+        diffy_result = Diffy::Diff.new(old_val, new_val, context: 0)
+        newline_alerts = diffy_result.count { |line| line.strip == '\\ No newline at end of file' }
+        diffy_result.each do |line|
           if regex.match(line.strip)
             return true if operator == '=~>'
           elsif operator == '=&>'
+            next if line.strip == '\\ No newline at end of file' && newline_alerts == 2
             return false
           end
         end

spec/octocatalog-diff/tests/catalog-diff/differ_spec.rb

@@ -1391,4 +1391,134 @@
       expect(result[0]).to eq(['!', "Class\fOpenssl::Package\fparameters\fcommon-array", [1, 2, 3], [1, 5, 25], fileref, fileref])
     end
   end
+
+  describe '#regexp_operator_match?' do
+    let(:subject) { described_class.allocate }
+
+    context 'for a multi-line diff' do
+      context 'for operator =~>' do
+        let(:operator) { '=~>' }
+        let(:regex) { Regexp.new('\\A.(kittens|cats)\\z') }
+
+        it 'should return true when at least one line matches' do
+          old_val = "kittens\nkittens\ncats\n"
+          new_val = "puppies\ndogs\n"
+          expect(subject.send(:regexp_operator_match?, operator, regex, old_val, new_val)).to eq(true)
+        end
+
+        it 'should return false when neither line matches' do
+          old_val = "puppies\ndogs\ndonkeys\n"
+          new_val = "puppies\ndogs\n"
+          expect(subject.send(:regexp_operator_match?, operator, regex, old_val, new_val)).to eq(false)
+        end
+      end
+
+      context 'for operator =&>' do
+        let(:operator) { '=&>' }
+        let(:regex) { Regexp.new('\\A(-|\\+)(kittens )*(kittens|cats)\\z') }
+
+        it 'should return true when all lines match' do
+          old_val = "kittens\nkittens\ncats\n"
+          new_val = "kittens\nkittens\n"
+          expect(subject.send(:regexp_operator_match?, operator, regex, old_val, new_val)).to eq(true)
+        end
+
+        it 'should return false when the regex does not match line' do
+          old_val = "kittens\nkittens\ncats\n"
+          new_val = "kittens\nkittens\ndogs\n"
+          expect(subject.send(:regexp_operator_match?, operator, regex, old_val, new_val)).to eq(false)
+        end
+
+        it 'should return true if both old and new do not end in a newline' do
+          old_val = "kittens\nkittens\ncats"
+          new_val = "kittens\nkittens\nkittens"
+          expect(subject.send(:regexp_operator_match?, operator, regex, old_val, new_val)).to eq(true)
+        end
+
+        it 'should return false if one ends in a newline and the other does not' do
+          old_val = "kittens\nkittens\ncats\n"
+          new_val = "kittens\nkittens\nkittens"
+          expect(subject.send(:regexp_operator_match?, operator, regex, old_val, new_val)).to eq(false)
+        end
+      end
+    end
+
+    context 'for a single-line diff' do
+      context 'for operator =~>' do
+        let(:operator) { '=~>' }
+        let(:regex) { Regexp.new('\\A(-|\\+)(kittens )+(kittens|cats)\\z') }
+
+        it 'should return true when at least one line matches' do
+          old_val = 'kittens kittens kittens'
+          new_val = 'kittens cats'
+          expect(subject.send(:regexp_operator_match?, operator, regex, old_val, new_val)).to eq(true)
+        end
+
+        it 'should return false when neither line matches' do
+          old_val = 'kittens dogs cats kittens kittens'
+          new_val = 'kittens cats dogs'
+          expect(subject.send(:regexp_operator_match?, operator, regex, old_val, new_val)).to eq(false)
+        end
+      end
+
+      context 'for operator =&>' do
+        let(:operator) { '=&>' }
+        let(:regex) { Regexp.new('\\A(-|\\+)(kittens )+(kittens|cats)\\z') }
+
+        it 'should return true when both lines match' do
+          old_val = 'kittens kittens kittens'
+          new_val = 'kittens cats'
+          expect(subject.send(:regexp_operator_match?, operator, regex, old_val, new_val)).to eq(true)
+        end
+
+        it 'should return false when the regex does not match line' do
+          old_val = 'kittens kittens dogs kittens'
+          new_val = 'kittens cats'
+          expect(subject.send(:regexp_operator_match?, operator, regex, old_val, new_val)).to eq(false)
+        end
+      end
+    end
+
+    context 'for a multi-line versus a single-line diff' do
+      context 'for operator =~>' do
+        let(:operator) { '=~>' }
+        let(:regex) { Regexp.new('\\A.(kittens|cats)\\z') }
+
+        it 'should return true when at least one line matches' do
+          old_val = "kittens\nkittens\ncats\n"
+          new_val = 'puppies'
+          expect(subject.send(:regexp_operator_match?, operator, regex, old_val, new_val)).to eq(true)
+        end
+
+        it 'should return false when neither line matches' do
+          old_val = "puppies\ndogs\ndonkeys\n"
+          new_val = 'puppies'
+          expect(subject.send(:regexp_operator_match?, operator, regex, old_val, new_val)).to eq(false)
+        end
+      end
+
+      context 'for operator =&>' do
+        let(:operator) { '=&>' }
+        let(:regex) { Regexp.new('\\A(-|\\+)(kittens )*(kittens|cats)\\z') }
+
+        it 'should return true when all lines match and both old and new do not end in newline' do
+          old_val = "kittens\nkittens\ncats"
+          new_val = 'kittens'
+          expect(subject.send(:regexp_operator_match?, operator, regex, old_val, new_val)).to eq(true)
+        end
+
+        it 'should return false when all lines match but ending in newlines differs' do
+          old_val = "kittens\nkittens\ncats\n"
+          new_val = 'kittens'
+          expect(subject.send(:regexp_operator_match?, operator, regex, old_val, new_val)).to eq(false)
+        end
+
+        it 'should return false when the regex does not match line' do
+          old_val = "kittens\nkittens\ncats"
+          new_val = 'dogs'
+          expect(subject.send(:regexp_operator_match?, operator, regex, old_val, new_val)).to eq(false)
+        end
+      end
+    end
+  end
 end