Resolve multiple issues for 1.4

-   Resolve ldiff output issues: Resolves #5 and #6 by adding system-output
    comparison calls to `bin/ldiff` compared against some pre-generated output.
    There is some timestamp manipulation involved with the output comparison,
    as the timestamps are unstable because of the way that git clone works.

-   Resolved a problem with bin/ldiff --context output.

-   Resolved a Numeric/Integer OptParse issue: later versions of Ruby had
    problems working with an `OptParse` option specification of `Numeric`; this
    has been changed to `Integer`.

.rubocop.yml

@@ -53,6 +53,7 @@ Style/BlockDelimiters:
     - trace
     - assert_raises
     - spec
+    - capture_subprocess_io
   FunctionalMethods:
     - let
     - subject

History.md

@@ -1,11 +1,18 @@
-## 1.NEXT / 2019-MM-DD
+## 1.4 / 2019-MM-DD
 
 *   Ruby versions lower than 2.3 are soft-deprecated and will not be run as
     part of the CI process any longer.
 *   Akinora MUSHA (knu) added the ability for Diff::LCS::Change objects to be
     implicitly treated arrays. Originally provided as pull request [#47][],
     but it introduced a number of test failures as documented in [#48][], and
     remediation of Diff::LCS itself was introduced in [#49][].
+*   Resolved [#5][] with some tests comparing output from `system` calls to
+    `bin/ldiff` with some pre-generated output. Resolved [#6][] with these
+    tests.
+*   Resolved a previously undetected `bin/ldiff` issue with `--context` output
+    not matching `diff --context` output.
+*   Resolved an issue with later versions of Ruby not working with an `OptParse`
+    specification of `Numeric`; this has been changed to `Integer`.
 
 ## 1.3 / 2017-01-18
 
@@ -212,6 +219,8 @@
 [#2]: https://github.com/halostatue/diff-lcs/issues/2
 [#3]: https://github.com/halostatue/diff-lcs/issues/3
 [#4]: https://github.com/halostatue/diff-lcs/issues/4
+[#5]: https://github.com/halostatue/diff-lcs/issues/5
+[#6]: https://github.com/halostatue/diff-lcs/issues/6
 [#8]: https://github.com/halostatue/diff-lcs/pull/8
 [#9]: https://github.com/halostatue/diff-lcs/pull/9
 [#10]: https://github.com/halostatue/diff-lcs/pull/10

Manifest.txt

@@ -25,8 +25,15 @@ lib/diff/lcs/ldiff.rb
 lib/diff/lcs/string.rb
 spec/change_spec.rb
 spec/diff_spec.rb
+spec/fixtures/aX
+spec/fixtures/bXaX
 spec/fixtures/ds1.csv
 spec/fixtures/ds2.csv
+spec/fixtures/ldiff/output.diff
+spec/fixtures/ldiff/output.diff-c
+spec/fixtures/ldiff/output.diff-e
+spec/fixtures/ldiff/output.diff-f
+spec/fixtures/ldiff/output.diff-u
 spec/hunk_spec.rb
 spec/issues_spec.rb
 spec/lcs_spec.rb

lib/diff/lcs/hunk.rb

@@ -20,6 +20,7 @@ def initialize(data_old, data_new, piece, flag_context, file_length_difference)
     before = after = file_length_difference
     after += @blocks[0].diff_size
     @file_length_difference = after # The caller must get this manually
+    @max_diff_size = @blocks.lazy.map { |e| e.diff_size }.max
 
     # Save the start & end of each array. If the array doesn't exist (e.g.,
     # we're only adding items in this block), then figure out the line
@@ -70,6 +71,8 @@ def flag_context=(context) #:nodoc: # rubocop:disable Lint/DuplicateMethods
         context
       end
 
+    add_end = @max_diff_size if add_end > @max_diff_size
+
     @end_old += add_end
     @end_new += add_end
   end
@@ -190,7 +193,7 @@ def context_diff
 
       removes.each do |block|
         block.remove.each do |item|
-          outlist[item.position - lo][0, 1] = encode(block.op) # - or !
+          outlist[item.position - lo].insert(0, encode(block.op)) # - or !
         end
       end
       s << outlist.join("\n")
@@ -203,7 +206,7 @@ def context_diff
       outlist = @data_new[lo..hi].collect { |e| e.insert(0, encode(' ')) }
       inserts.each do |block|
         block.insert.each do |item|
-          outlist[item.position - lo][0, 1] = encode(block.op) # + or !
+          outlist[item.position - lo].insert(0, encode(block.op)) # - or !
         end
       end
       s << outlist.join("\n")

lib/diff/lcs/ldiff.rb

@@ -30,14 +30,14 @@ def run(args, _input = $stdin, output = $stdout, error = $stderr) #:nodoc:
       o.banner = "Usage: #{File.basename($0)} [options] oldfile newfile"
       o.separator ''
       o.on(
-        '-c', '-C', '--context [LINES]', Numeric,
+        '-c', '-C', '--context [LINES]', Integer,
         'Displays a context diff with LINES lines', 'of context. Default 3 lines.'
       ) do |ctx|
         @format = :context
         @lines  = ctx || 3
       end
       o.on(
-        '-u', '-U', '--unified [LINES]', Numeric,
+        '-u', '-U', '--unified [LINES]', Integer,
         'Displays a unified diff with LINES lines', 'of context. Default 3 lines.'
       ) do |ctx|
         @format = :unified
@@ -134,9 +134,9 @@ def run(args, _input = $stdin, output = $stdout, error = $stderr) #:nodoc:
     end
 
     if (@format == :unified) or (@format == :context)
-      ft = File.stat(file_old).mtime.localtime.strftime('%Y-%m-%d %H:%M:%S.%N %z')
+      ft = File.stat(file_old).mtime.localtime.strftime('%Y-%m-%d %H:%M:%S.000000000 %z')
       output << "#{char_old} #{file_old}\t#{ft}\n"
-      ft = File.stat(file_new).mtime.localtime.strftime('%Y-%m-%d %H:%M:%S.%N %z')
+      ft = File.stat(file_new).mtime.localtime.strftime('%Y-%m-%d %H:%M:%S.000000000 %z')
       output << "#{char_new} #{file_new}\t#{ft}\n"
     end
 
@@ -155,15 +155,18 @@ def run(args, _input = $stdin, output = $stdout, error = $stderr) #:nodoc:
         file_length_difference = hunk.file_length_difference
 
         next unless oldhunk
-        next if @lines.postive? and hunk.merge(oldhunk)
+        next if @lines.positive? and hunk.merge(oldhunk)
 
         output << oldhunk.diff(@format) << "\n"
       ensure
         oldhunk = hunk
       end
     end
 
-    output << oldhunk.diff(@format) << "\n"
+    last = oldhunk.diff(@format)
+    last << "\n" if last.respond_to?(:end_with?) && !last.end_with?("\n")
+
+    output << last
 
     output.reverse_each { |e| real_output << e.diff(:ed_finish) } if @format == :ed
 

spec/fixtures/aX

@@ -0,0 +1 @@
+aX

spec/fixtures/bXaX

@@ -0,0 +1 @@
+bXaX

spec/fixtures/ldiff/output.diff

@@ -0,0 +1,4 @@
+1c1
+< aX
+---
+> bXaX

spec/fixtures/ldiff/output.diff-c

@@ -0,0 +1,7 @@
+*** spec/fixtures/aX	2019-02-01 22:29:34.000000000 -0500
+--- spec/fixtures/bXaX	2019-02-01 22:29:43.000000000 -0500
+***************
+*** 1 ****
+! aX
+--- 1 ----
+! bXaX

spec/fixtures/ldiff/output.diff-e

@@ -0,0 +1,3 @@
+1c
+bXaX
+.

spec/fixtures/ldiff/output.diff-f

@@ -0,0 +1,3 @@
+c1
+bXaX
+.

spec/fixtures/ldiff/output.diff-u

@@ -0,0 +1,5 @@
+--- spec/fixtures/aX	2019-02-01 22:29:34.000000000 -0500
++++ spec/fixtures/bXaX	2019-02-01 22:29:43.000000000 -0500
+@@ -1 +1 @@
+-aX
++bXaX

spec/hunk_spec.rb

@@ -13,7 +13,7 @@
 
     it 'produces a unified diff from the two pieces' do
       expected = <<-EXPECTED.gsub(/^\s+/, '').encode('UTF-16LE').chomp
-        @@ -1,2 +1,2 @@
+        @@ -1 +1 @@
         -Tu avec carté {count} itém has
         +Tu avec carte {count} item has
       EXPECTED
@@ -24,18 +24,18 @@
     it 'produces a context diff from the two pieces' do
       expected = <<-EXPECTED.gsub(/^\s+/, '').encode('UTF-16LE').chomp
         ***************
-        *** 1,2 ****
-        !Tu avec carté {count} itém has
-        --- 1,2 ----
-        !Tu avec carte {count} item has
+        *** 1 ****
+        ! Tu avec carté {count} itém has
+        --- 1 ----
+        ! Tu avec carte {count} item has
       EXPECTED
 
       expect(hunk.diff(:context)).to eq(expected)
     end
 
     it 'produces an old diff from the two pieces' do
       expected = <<-EXPECTED.gsub(/^ +/, '').encode('UTF-16LE').chomp
-        1,2c1,2
+        1c1
         < Tu avec carté {count} itém has
         ---
         > Tu avec carte {count} item has
@@ -47,7 +47,7 @@
 
     it 'produces a reverse ed diff from the two pieces' do
       expected = <<-EXPECTED.gsub(/^ +/, '').encode('UTF-16LE').chomp
-        c1,2
+        c1
         Tu avec carte {count} item has
         .
 

spec/ldiff_spec.rb

@@ -2,50 +2,71 @@
 
 require 'spec_helper'
 
-describe 'Diff::LCS.diff' do
-  include Diff::LCS::SpecHelper::Matchers
+RSpec.describe 'bin/ldiff' do
+  include CaptureSubprocessIO
 
-  it 'correctly diffs seq1 to seq2' do
-    diff_s1_s2 = Diff::LCS.diff(seq1, seq2)
-    expect(change_diff(correct_forward_diff)).to eq(diff_s1_s2)
+  let(:output_diff) { read_fixture }
+  let(:output_diff_c) { read_fixture('-c') }
+  let(:output_diff_e) { read_fixture('-e') }
+  let(:output_diff_f) { read_fixture('-f') }
+  let(:output_diff_u) { read_fixture('-u') }
+
+  specify do
+    expect(run_ldiff).to eq(output_diff)
   end
 
-  it 'correctly diffs seq2 to seq1' do
-    diff_s2_s1 = Diff::LCS.diff(seq2, seq1)
-    expect(change_diff(correct_backward_diff)).to eq(diff_s2_s1)
+  specify do
+    expect(run_ldiff('-c')).to eq(output_diff_c)
   end
 
-  it 'correctly diffs against an empty sequence' do
-    diff = Diff::LCS.diff(word_sequence, [])
-    correct_diff = [
-      [
-        ['-', 0, 'abcd'],
-        ['-', 1, 'efgh'],
-        ['-', 2, 'ijkl'],
-        ['-', 3, 'mnopqrstuvwxyz']
-      ]
-    ]
+  specify do
+    expect(run_ldiff('-e')).to eq(output_diff_e)
+  end
 
-    expect(change_diff(correct_diff)).to eq(diff)
+  specify do
+    expect(run_ldiff('-f')).to eq(output_diff_f)
+  end
 
-    diff = Diff::LCS.diff([], word_sequence)
-    correct_diff.each do |hunk|
-      hunk.each do |change| change[0] = '+' end
-    end
-    expect(change_diff(correct_diff)).to eq(diff)
+  specify do
+    expect(run_ldiff('-u')).to eq(output_diff_u)
   end
 
-  it "correctly diffs 'xx' and 'xaxb'" do
-    left = 'xx'
-    right = 'xaxb'
-    expect(Diff::LCS.patch(left, Diff::LCS.diff(left, right))).to eq(right)
+  def read_fixture(flag = nil)
+    clean_data(IO.binread("spec/fixtures/ldiff/output.diff#{flag}"), flag)
   end
 
-  it 'returns an empty diff with (hello, hello)' do
-    expect(Diff::LCS.diff(hello, hello)).to eq([])
+  def clean_data(data, flag)
+    case flag
+    when '-c', '-u'
+      clean_output_timestamp(data)
+    else
+      data
+    end
+  end
+
+  def clean_output_timestamp(data)
+    data.gsub(
+      %r{
+        [-*+]{3}
+        \s
+        spec/fixtures/(\w+)
+        \s
+        \d{4}-\d\d-\d\d
+        \s
+        \d\d:\d\d:\d\d(?:\.\d+)
+        \s
+        (?:[-+]\d{4}|Z)
+      }x,
+      '*** spec/fixtures/\1	0000-00-00 00:00:00.000000000 -0000'
+    )
   end
 
-  it 'returns an empty diff with (hello_ary, hello_ary)' do
-    expect(Diff::LCS.diff(hello_ary, hello_ary)).to eq([])
+  def run_ldiff(flag = nil, left: 'aX', right: 'bXaX')
+    stdout, stderr = capture_subprocess_io do
+      system("ruby -Ilib bin/ldiff #{flag} spec/fixtures/#{left} spec/fixtures/#{right}")
+    end
+    expect(stderr).to be_empty
+    expect(stdout).not_to be_empty
+    clean_data(stdout, flag)
   end
 end

spec/spec_helper.rb

@@ -40,6 +40,38 @@ def require_do(resource)
 
 $:.unshift parent.join('lib')
 
+module CaptureSubprocessIO
+  def _synchronize
+    yield
+  end
+
+  def capture_subprocess_io
+    _synchronize do
+      begin
+        require 'tempfile'
+
+        captured_stdout, captured_stderr = Tempfile.new('out'), Tempfile.new('err')
+
+        orig_stdout, orig_stderr = $stdout.dup, $stderr.dup
+        $stdout.reopen captured_stdout
+        $stderr.reopen captured_stderr
+
+        yield
+
+        $stdout.rewind
+        $stderr.rewind
+
+        return captured_stdout.read, captured_stderr.read
+      ensure
+        captured_stdout.unlink
+        captured_stderr.unlink
+        $stdout.reopen orig_stdout
+        $stderr.reopen orig_stderr
+      end
+    end
+  end
+end
+
 require 'diff-lcs'
 
 module Diff::LCS::SpecHelper