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

CollectionSerializer tests aren't real #1617

Closed
bf4 opened this issue Mar 25, 2016 · 0 comments
Closed

CollectionSerializer tests aren't real #1617

bf4 opened this issue Mar 25, 2016 · 0 comments

Comments

@bf4
Copy link
Member

bf4 commented Mar 25, 2016

Expected behavior vs actual behavior

While working on following up to #1537 I noticed:

The CollectionSerializer should only ever have in it a collection, such as an Array or ActiveRecord::Relation. i.e. something that responds to :each

The CollectionSerializer has a fallback for the collection root/json_key object.try(:name) (where object is the collection of resources based in).

The tests since #1007 and #1013 have monkey patched object to have the method name.

If we change object.try(:name) to object.try!(:name) we raise an exception since a regular Array [] does not have a name method, and try! which checks if the receiver responds to the method.

However, the code does not ensure that the object is actually responds to name. e.g. an Array doesn't have a name, but the tests use a helper build_named_collection that returns an monkey-patched array [] that has a [].name #=> MeResource

This failure is hidden by the try methods.

        key = serializers.first.try(:json_key) || object.try(:name).try(:underscore)
        key.try(:pluralize)

To make the interface clear, it would be best to remove the try methods altogether

Steps to reproduce

(e.g., detailed walkthrough, runnable script, example application)

Environment

ActiveModelSerializers Version (commit ref if not on tag):

0.10.0.rc4, git rev-parse master 61412d8

Output of ruby -e "puts RUBY_DESCRIPTION":

ruby 2.2.3p173 (2015-08-18 revision 51636) [x86_64-darwin14]

OS Type & Version:

Mac 10.10.5 Yosemite

Integrated application and version (e.g., Rails, Grape, etc):

n/a

Backtrace

(e.g., provide any applicable backtraces from your application)

Additonal helpful information

(e.g., Gemfile.lock, configurations, PR containing a failing test, git bisect results)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant