Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cucumber::Ast::Table.diff! is broken when using no headers #278

Closed
troessner opened this issue May 10, 2012 · 10 comments
Closed

Cucumber::Ast::Table.diff! is broken when using no headers #278

troessner opened this issue May 10, 2012 · 10 comments
Milestone

Comments

@troessner
Copy link

I have the following lines in one of my scenarios:

And I should see locations table                                             
  | Name    |      |      |
  | Berlin  | Show | Edit |
  | Hamburg | Show | Edit |

The step definition for this is (debugger added, see later):

Then(/^I should see (.+) table$/) do |table_id, expected_table|
  html_table = table_at("##{table_id.parameterize.tableize}").to_a
  html_table.map! { |r| r.map! { |c| c.gsub(/<.+?>/, '').gsub(/[\n\t\r]/, '') } }
debugger
  expected_table.diff!(html_table)
end

Now, my table does exactly look like specified above (more no this later), but the step still fails:

Tables were not identical (Cucumber::Ast::Table::Different)
./features/step_definitions/common_steps.rb:18:in `/^I should see (.+) table$/'
features/manage_locations.feature:15:in `And I should see locations table'

The problem here is that some of the columns have an empty header and that's where the diff! method falls short.
Using the debugger I can see the following:

<(rdb:1) p expected_table.raw
[["Name", "", ""], ["Berlin", "Show", "Edit"], ["Hamburg", "Show", "Edit"]]
(rdb:1) p html_table
[["Name", "", ""], ["Berlin", "Show", "Edit"], ["Hamburg", "Show", "Edit"]]
(rdb:1) p expected_table.raw == html_table
true

So, as you can see, the "raw" representation of the expected_table is the very same as of the html_table but the step still fails.

I think there are two things wrong here:

1.) IMHO it shouldn't fail since the tables are identical regarding everything I can see.
2.) At least the error message should tell me what exactly is not identical. From the output above I have no chance to fix this.

I looked into the implementation of the diff!-method, but unfortunately this isn't really a beauty (no offense), so it's definitely no 5 minute job to improve this. (To be honest, I think the whole "diff!" method needs a huge refactoring - it's way to long, nothing is really abstract or loosely coupled and it's badly readable.)

Possible cause:

I think the problem is that the method calls hashes internally, which uses the table headers as keys, if more than one table header is not set -> boom, there goes one column.

A colleague of mine created a sample project to illustrate this: https://github.com/exviva/cucumber_table_diff

What do you think?
Am I missing something here?

@ramgole
Copy link

ramgole commented Jul 27, 2012

This also happens if 2 headers are the same like

  | Name    | X    | X    |
  | Berlin  | Show | Edit |
  | Hamburg | Show | Edit |

@ramgole
Copy link

ramgole commented Jul 27, 2012

I avoided this problem by calling transpose on both tables before taking diff!

@os97673
Copy link
Member

os97673 commented Mar 27, 2013

@robertodecurnex are you going to work on this issue?

@robertodecurnex
Copy link
Contributor

Yea, I'll be digging over this.

There are conceptual issues in the middle too :S

I'll try to figure it out and maybe jump into an IRC discussion if I can not.

@mattwynne
Copy link
Member

Thanks @robertodecurnex!

@mattwynne
Copy link
Member

@robertodecurnex how are you getting on? Need any help?

@mattwynne
Copy link
Member

Closing due to inactivity. Please reopen if and when you have a fix.

@betelgeuse
Copy link

@mattwynne wouldn't it be better to keep bugs open so users are more likely to find them and pick them up to work on them?

@mattwynne
Copy link
Member

@betelgeuse I don't like having bugs hanging around not being fixed. To me, this one is an edge-case that nobody on the core team is going to take care of. So I've closed it because if someone cares about it, they can re-open it and work on it.

But maybe you're right, and that's a confusing way for us to operate.

Would you like to work on this yourself?

roschaefer pushed a commit to roschaefer/cucumber that referenced this issue Apr 10, 2015
roschaefer pushed a commit to roschaefer/cucumber that referenced this issue Apr 10, 2015
roschaefer pushed a commit to roschaefer/cucumber that referenced this issue Apr 10, 2015
mattwynne pushed a commit that referenced this issue Apr 20, 2015
@lock
Copy link

lock bot commented Oct 25, 2018

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.

@lock lock bot locked as resolved and limited conversation to collaborators Oct 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants