Diffing empty DataTables crashes with a nil error

[botandrose]

Current Behavior

Opinions seem to vary on exactly how to construct an empty (0x0) DataTable, but in either case:

2.2.6 :001 > require 'cucumber'
2.2.6 :002 > include Cucumber::MultilineArgument
2.2.6 :003 > DataTable.from([[]]).diff! [[]]
NoMethodError: undefined method `length' for nil:NilClass
	from /home/micah/.rvm/gems/ruby-2.2.6@chop/gems/cucumber-2.4.0/lib/cucumber/multiline_argument/data_table.rb:357:in `diff!'
2.2.6 :004 > DataTable.from([]).diff! []
NoMethodError: undefined method `length' for nil:NilClass
	from /home/micah/.rvm/gems/ruby-2.2.6@chop/gems/cucumber-2.4.0/lib/cucumber/multiline_argument/data_table.rb:355:in `diff!'

This ends up boiling down to C::MA::DT#pad_and_match calling Array#transpose on the internal 2d array, and both [[]].transpose and [].transpose both return [], which causes the DataTable implementation to crash with a nil error, because its expecting a 2d array, but it only has a 1d array to work with.

Expected Behavior

One might argue that the bug is truly in Ruby's Array#transpose function (confirmed same behavior in ruby-head, btw), but regardless, I think we can agree that #diff! shouldn't nil crash in this circumstance. I would like to see at least one of those two invocations succeed, and possibly both.

Context & Motivation

The motivation behind this request is that I am heavily using DataTables in my cucumber scenarios, and have even written a library named chop that makes DataTables much more powerful when used in conjunction with ActiveRecord and Capybara. The specific use-case is as follows:

I have a method that creates a data structure based off of the current state of the Capybara page, lets say its a table of users, and I want to use the #diff! method to assert that it matches an expected state, which could include being completely empty.

Then "I should see the following users:" |table|
  table.diff! users_table
end

Then "I should see no users"
  empty_table.diff! users_table
end

def users_table
  all("table.users tr").map do |row|
    row.all("td").map do |cell|
      cell.text
    end
  end
end

def empty_table
  Cucumber::MultilineArgument::DataTable.from([[]]) # or [], perhaps
end

This may seem a bit awkward, like why not just do expect(users_table).to eq([]), but this is a gross simplification of the actual system. Suffice it to say that using C::MA::DT#diff! in both circumstances is greatly preferable. One could look at the implementation and usage of Chop::Diff for a better understanding of the actual system, if desired.

Next Steps

Would a PR addressing this issue be considered for merging? If so, I am happy and eager to do the work.

[botandrose]

As an aside, it seems that Cucumber presently has a preference of [[]] as an empty table.

2.2.6 :001 > require 'cucumber'
2.2.6 :002 > include Cucumber::MultilineArgument
2.2.6 :003 > DataTable.from([["A","B"],["1","2"],["3","4"]]).diff! [[]]
Cucumber::MultilineArgument::DataTable::Different: Tables were not identical:

  | (-) A | (-) B |
  | (-) 1 | (-) 2 |
  | (-) 3 | (-) 4 |

2.2.6 :004 > DataTable.from([["A","B"],["1","2"],["3","4"]]).diff! []
NoMethodError: undefined method `value=' for nil:NilClass
	from /home/micah/.rvm/gems/ruby-2.2.6@chop/gems/cucumber-2.4.0/lib/cucumber/multiline_argument/data_table.rb:594:in `block in pad_and_match'
	from /home/micah/.rvm/gems/ruby-2.2.6@chop/gems/cucumber-2.4.0/lib/cucumber/multiline_argument/data_table.rb:587:in `each'
	from /home/micah/.rvm/gems/ruby-2.2.6@chop/gems/cucumber-2.4.0/lib/cucumber/multiline_argument/data_table.rb:587:in `each_with_index'
	from /home/micah/.rvm/gems/ruby-2.2.6@chop/gems/cucumber-2.4.0/lib/cucumber/multiline_argument/data_table.rb:587:in `pad_and_match'
	from /home/micah/.rvm/gems/ruby-2.2.6@chop/gems/cucumber-2.4.0/lib/cucumber/multiline_argument/data_table.rb:356:in `diff!'

FWIW.

[botandrose]

Also, I think this last part shows that since Cucumber thinks
DataTable.from([["A","B"],["1","2"],["3","4"]]).diff! [[]] is a reasonable assertion, then thereforeDataTable.from([[]]).diff! [[]] should be reasonable, as well.

Thoughts?

[danascheider]

Thank you for the issue report and PR, @botandrose! I'm going to take a look at this after I get out of my current meeting but wanted to respond right away :)

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.