Skip to content

make "the JSON should have the following" not skip the first assertion #33

Merged
merged 1 commit into from Mar 19, 2013

4 participants

@levinalex

when using the Then the JSON should have the following-assertion in cucumber like so:

Then the JSON should have the following:
  | string  | "string  |
  | integer | 42       |

the first line of the table was interpreted as a table header and skipped. The attached patch fixes that.

I don't really know how to write a proper test for that.

@laserlemon
Collective Idea member

Thank you! 👏

Could you take a stab at writing a test for this? What comes to mind is duplicating how you came across this bug. Write a table that should fail (and wouldn't have before your fix) and assert that it does indeed fail. Does that make sense? If not, I can try later, just not now. 😜 Thanks again.

@laserlemon
Collective Idea member

Any progress on this? Thanks.

@vocaro
vocaro commented Dec 10, 2012

Could this patch be applied? The issue bit me as well, since the README is completely wrong. The code assumes there is a table header -- because it iterates over table.rows -- while the README examples have no header. Either the README examples need to have a table header, or the code needs to be changed to use table.rows_hash.

@laserlemon
Collective Idea member

@vocaro Not without tests. If you could write a test surrounding the fix, that'd be wonderful. Otherwise, I'll get to it when I can. Thank you!

@clabrunda

+1 for merging this.

I'm not sure there's a good way to write a test for this fix. The only thing I could come up with was to add a tag like @wip (maybe @fail) that makes the scenario pass only if it fails. But that seems to violate the spirit of Given - When - Then.

@cbusbey cbusbey referenced this pull request Mar 15, 2013
Merged

CI changes for Issue 33 #39

@laserlemon laserlemon merged commit c9b620c into collectiveidea:master Mar 19, 2013

1 check passed

Details default The Travis build passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.