From 15169228be42559f98fa6729d5f6bb32edad44e3 Mon Sep 17 00:00:00 2001 From: Austin Ziegler Date: Sat, 2 Feb 2019 22:52:28 -0500 Subject: [PATCH] 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 | 1 + History.md | 11 +++- Manifest.txt | 7 +++ lib/diff/lcs/hunk.rb | 7 ++- lib/diff/lcs/ldiff.rb | 15 +++--- spec/fixtures/aX | 1 + spec/fixtures/bXaX | 1 + spec/fixtures/ldiff/output.diff | 4 ++ spec/fixtures/ldiff/output.diff-c | 7 +++ spec/fixtures/ldiff/output.diff-e | 3 ++ spec/fixtures/ldiff/output.diff-f | 3 ++ spec/fixtures/ldiff/output.diff-u | 5 ++ spec/hunk_spec.rb | 14 ++--- spec/ldiff_spec.rb | 85 +++++++++++++++++++------------ spec/spec_helper.rb | 32 ++++++++++++ 15 files changed, 148 insertions(+), 48 deletions(-) create mode 100644 spec/fixtures/aX create mode 100644 spec/fixtures/bXaX create mode 100644 spec/fixtures/ldiff/output.diff create mode 100644 spec/fixtures/ldiff/output.diff-c create mode 100644 spec/fixtures/ldiff/output.diff-e create mode 100644 spec/fixtures/ldiff/output.diff-f create mode 100644 spec/fixtures/ldiff/output.diff-u diff --git a/.rubocop.yml b/.rubocop.yml index de2cbc6..9768185 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -53,6 +53,7 @@ Style/BlockDelimiters: - trace - assert_raises - spec + - capture_subprocess_io FunctionalMethods: - let - subject diff --git a/History.md b/History.md index 11afd6d..fb0e99c 100644 --- a/History.md +++ b/History.md @@ -1,4 +1,4 @@ -## 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. @@ -6,6 +6,13 @@ 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 diff --git a/Manifest.txt b/Manifest.txt index f583076..5c33429 100644 --- a/Manifest.txt +++ b/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 diff --git a/lib/diff/lcs/hunk.rb b/lib/diff/lcs/hunk.rb index 58a68db..4ec5123 100644 --- a/lib/diff/lcs/hunk.rb +++ b/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") diff --git a/lib/diff/lcs/ldiff.rb b/lib/diff/lcs/ldiff.rb index d385f72..2862267 100644 --- a/lib/diff/lcs/ldiff.rb +++ b/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,7 +155,7 @@ 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 @@ -163,7 +163,10 @@ def run(args, _input = $stdin, output = $stdout, error = $stderr) #:nodoc: 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 diff --git a/spec/fixtures/aX b/spec/fixtures/aX new file mode 100644 index 0000000..5765d6a --- /dev/null +++ b/spec/fixtures/aX @@ -0,0 +1 @@ +aX diff --git a/spec/fixtures/bXaX b/spec/fixtures/bXaX new file mode 100644 index 0000000..a1c813d --- /dev/null +++ b/spec/fixtures/bXaX @@ -0,0 +1 @@ +bXaX diff --git a/spec/fixtures/ldiff/output.diff b/spec/fixtures/ldiff/output.diff new file mode 100644 index 0000000..fa1a347 --- /dev/null +++ b/spec/fixtures/ldiff/output.diff @@ -0,0 +1,4 @@ +1c1 +< aX +--- +> bXaX diff --git a/spec/fixtures/ldiff/output.diff-c b/spec/fixtures/ldiff/output.diff-c new file mode 100644 index 0000000..92ea5a2 --- /dev/null +++ b/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 diff --git a/spec/fixtures/ldiff/output.diff-e b/spec/fixtures/ldiff/output.diff-e new file mode 100644 index 0000000..13e0f7f --- /dev/null +++ b/spec/fixtures/ldiff/output.diff-e @@ -0,0 +1,3 @@ +1c +bXaX +. diff --git a/spec/fixtures/ldiff/output.diff-f b/spec/fixtures/ldiff/output.diff-f new file mode 100644 index 0000000..77710c7 --- /dev/null +++ b/spec/fixtures/ldiff/output.diff-f @@ -0,0 +1,3 @@ +c1 +bXaX +. diff --git a/spec/fixtures/ldiff/output.diff-u b/spec/fixtures/ldiff/output.diff-u new file mode 100644 index 0000000..c4ba30f --- /dev/null +++ b/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 diff --git a/spec/hunk_spec.rb b/spec/hunk_spec.rb index 5a45072..277f739 100644 --- a/spec/hunk_spec.rb +++ b/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,10 +24,10 @@ 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) @@ -35,7 +35,7 @@ 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 . diff --git a/spec/ldiff_spec.rb b/spec/ldiff_spec.rb index 113c0dd..6dfea29 100644 --- a/spec/ldiff_spec.rb +++ b/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 diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 4899c49..e566d0e 100644 --- a/spec/spec_helper.rb +++ b/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