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.

Sign up for GitHub

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

Fix for issue: #1199: Changed DataTable#symbolic_hashes to not change the state of the table #1200

Merged
merged 1 commit into from
Sep 21, 2017
Merged

Fix for issue: #1199: Changed DataTable#symbolic_hashes to not change the state of the table #1200

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions lib/cucumber/multiline_argument/data_table.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,10 @@ def hashes
# [{:foo => '2', :bar => '3', :foo_bar => '5'}, {:foo => '7', :bar => '9', :foo_bar => '16'}]
#
def symbolic_hashes
@header_conversion_proc = lambda { |h| symbolize_key(h) }
@symbolic_hashes ||= build_hashes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this build_hashes method used elsewhere? If not I'd delete it. Same with the @header_conversion_proc - make sure it isn't used elsewhere before deleting it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • build_hashes is called above on line 147 within the method hashes, which in turn is called on line 163.
  • @header_conversion_proc is used elsewhere and the modification to this variable in @symbolic_hashes is what is/was causing the issue.

Personally, I wouldn't mind removing usage of @header_conversion_proc throughout the file as it seems a bit unnecessary. However, I think that is outside the scope of this bugfix.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, thanks for clarifying!

@symbolic_hashes ||=
self.hashes.map do |string_hash|
Hash[string_hash.map{ |a,b| [symbolize_key(a), b] }]
end
end

# Converts this table into a Hash where the first column is
Expand Down
5 changes: 5 additions & 0 deletions spec/cucumber/multiline_argument/data_table_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,11 @@ module MultilineArgument
expect{@table.symbolic_hashes}.to_not raise_error
end

it 'should not interfere with use of #hashes' do
@table.symbolic_hashes
expect{@table.hashes}.to_not raise_error
end

end

describe '#map_column!' do
Expand Down