Calling fixture accessor method with no arguments returns all fixtures rather than empty array #28692

Merged
merged 1 commit into from Apr 7, 2017

Conversation

Projects
None yet
5 participants
@kmcphillips
Contributor

kmcphillips commented Apr 7, 2017

Problem

Fixtures provide a convenience method to access fixtures by name. So given:

# fixtures/things.yml
one:
  name: The first
two:
  name: The second

You can access the Things using things():

things(:two)
# #<Thing:0x007f821b6346c0 name: "The second">
things(:one, :two)
# [#<Thing:0x007f821b6346c8 name: "The first">, #<Thing:0x007f821b6346c0 name: "The second">]

The inconsistent behaviour is that passing no arguments to this method always returns an empty array:

things
# []

The reason this happens is that it initializes an empty array of results, then iterates over all names passed in and pushes them into that array. Any names that do not match raise StandardError, but we don't check for empty arguments.

Solution

We now return all records for this fixture if none are passed in:

things
# [#<Thing:0x007f821b6346c8 name: "The first">, #<Thing:0x007f821b6346c0 name: "The second">]

There is a special case for if there is only a single fixture that exists, and was call with no arguments, we still return the array with one element. Where as if we ask for a single fixture by name we return it without wrapping it in an array.

Alternative solution

A simpler solution would be:

raise StandardError, "Must pass in a name" if fixture_names.blank?

But the "return all" solution exposes a nice iteration functionality that could be useful.

@rafaelfranca

@rails-bot

This comment has been minimized.

Show comment
Hide comment
@rails-bot

rails-bot Apr 7, 2017

r? @sgrif

(@rails-bot has picked a reviewer for you, use r? to override)

r? @sgrif

(@rails-bot has picked a reviewer for you, use r? to override)

@kmcphillips

This comment has been minimized.

Show comment
Hide comment
@kmcphillips

kmcphillips Apr 7, 2017

Contributor

Worth noting that pushing all names into fixture_names rather than just early existing with @loaded_fixtures[fs_name].fixtures.values causes them to be cached by following the existing code path.

Contributor

kmcphillips commented Apr 7, 2017

Worth noting that pushing all names into fixture_names rather than just early existing with @loaded_fixtures[fs_name].fixtures.values causes them to be cached by following the existing code path.

@rafaelfranca rafaelfranca merged commit 2807986 into rails:master Apr 7, 2017

2 checks passed

codeclimate no new or fixed issues
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment