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

Bring back assert_serializer #1099

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ Features:
CollectionSerializer for clarity, add ActiveModelSerializers.config.collection_serializer (@bf4)
- [#1295](https://github.com/rails-api/active_model_serializers/pull/1295) Add config `serializer_lookup_enabled` that,
when disabled, requires serializers to explicitly specified. (@trek)
- [#1099](https://github.com/rails-api/active_model_serializers/pull/1099) Adds `assert_serializer` test helper (@maurogeorge)

Fixes:
- [#1239](https://github.com/rails-api/active_model_serializers/pull/1239) Fix duplicates in JSON API compound documents (@beauby)
Expand Down
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ version = ENV['RAILS_VERSION'] || '4.2'
if version == 'master'
gem 'rack', github: 'rack/rack'
gem 'arel', github: 'rails/arel'
gem 'rails-controller-testing', github: 'rails/rails-controller-testing'
git 'https://github.com/rails/rails.git' do
gem 'railties'
gem 'activesupport'
Expand Down
1 change: 1 addition & 0 deletions docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ This is the documentation of ActiveModelSerializers, it's focused on the **0.10.
- [How to add root key](howto/add_root_key.md)
- [How to add pagination links](howto/add_pagination_links.md)
- [Using ActiveModelSerializers Outside Of Controllers](howto/outside_controller_use.md)
- [Testing ActiveModelSerializers](howto/test.md)

## Integrations

Expand Down
29 changes: 29 additions & 0 deletions docs/howto/test.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# How to test

## Test helpers

ActiveModelSerializers provides a `assert_serializer` method to be used on your controller tests to
assert that a specific serializer was used.

```ruby
class PostsControllerTest < ActionController::TestCase
test "should render post serializer" do
get :index
assert_serializer "PostSerializer"
# # return a custom error message
# assert_serializer "PostSerializer", "PostSerializer not rendered"
#
# # assert that the instance of PostSerializer was rendered
# assert_serializer PostSerializer
#
# # assert that the "PostSerializer" serializer was rendered
# assert_serializer :post_serializer
#
# # assert that the rendered serializer starts with "Post"
# assert_serializer %r{\APost.+\Z}
#
# # assert that no serializer was rendered
# assert_serializer nil
end
end
```
4 changes: 4 additions & 0 deletions lib/active_model/serializer/railtie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,9 @@ class Railtie < Rails::Railtie
app.load_generators
require 'generators/serializer/resource_override'
end

if Rails.env.test?
ActionController::TestCase.send(:include, ActiveModelSerializers::Test::Serializer)
end
end
end
1 change: 1 addition & 0 deletions lib/active_model_serializers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ def self.config
autoload :Model
autoload :Callbacks
autoload :Logging
autoload :Test

module_function

Expand Down
5 changes: 5 additions & 0 deletions lib/active_model_serializers/test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module ActiveModelSerializers
module Test
autoload :Serializer, 'active_model_serializers/test/serializer'
end
end
122 changes: 122 additions & 0 deletions lib/active_model_serializers/test/serializer.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
module ActiveModelSerializers
module Test
module Serializer
extend ActiveSupport::Concern

included do
setup :setup_serialization_subscriptions
end

# Asserts that the request was rendered with the appropriate serializers.
#
# # assert that the "PostSerializer" serializer was rendered
# assert_serializer "PostSerializer"
#
# # return a custom error message
# assert_serializer "PostSerializer", "PostSerializer not rendered"
#
# # assert that the instance of PostSerializer was rendered
# assert_serializer PostSerializer
#
# # assert that the "PostSerializer" serializer was rendered
# assert_serializer :post_serializer
#
# # assert that the rendered serializer starts with "Post"
# assert_serializer %r{\APost.+\Z}
#
# # assert that no serializer was rendered
# assert_serializer nil
#
def assert_serializer(expectation, message = nil)
@assert_serializer.expectation = expectation
@assert_serializer.message = message
@assert_serializer.response = response
assert(@assert_serializer.matches?, @assert_serializer.message)
Copy link
Member

Choose a reason for hiding this comment

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

So, people using RSpec would need to define assert(expected, actual) and any library not using ActionController::TestCase would also need to include ActiveModelSerializers::Test::Serializer (I forget if RSpec-Rails uses this)

something like:

module AMSAssertSerializer
  extend ActiveSupport::Concern
  included do
    include ActiveModelSerializers::Test::Serializer
  end
  def assert(expected, actual)
    expect(actual).to eq(expected)
  end
end
RSpec.configuration.include AMSAssertSerializer, type: :controller

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the review.

I don't think we need to care with RSpec and other libs at the moment, Action Mailer uses Minitest methods and Active Job too.

I think we can follow the same rationale, we are providing Minitest test helpers.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I know. :)

end

class AssertSerializer
attr_reader :serializers, :message
attr_accessor :response, :expectation

def initialize
@serializers = []
end

def message=(message)
@message = message || "expecting <#{expectation.inspect}> but rendering with <#{serializers}>"
end

def matches?
# Force body to be read in case the template is being streamed.
response.body

case expectation
when a_serializer?
matches_class?
when Symbol
matches_symbol?
when String
matches_string?
when Regexp
matches_regexp?
when NilClass
matches_nil?
else
fail ArgumentError, 'assert_serializer only accepts a String, Symbol, Regexp, ActiveModel::Serializer, or nil'
end
end

def subscribe
ActiveSupport::Notifications.subscribe(event_name) do |_name, _start, _finish, _id, payload|
serializer = payload[:serializer].name
serializers << serializer
end
end

def unsubscribe
ActiveSupport::Notifications.unsubscribe(event_name)
end

private

def matches_class?
serializers.include?(expectation.name)
end

def matches_symbol?
camelize_expectation = expectation.to_s.camelize
serializers.include?(camelize_expectation)
end

def matches_string?
!expectation.empty? && serializers.include?(expectation)
end

def matches_regexp?
serializers.any? do |serializer|
serializer.match(expectation)
end
end

def matches_nil?
serializers.blank?
end

def a_serializer?
->(exp) { exp.is_a?(Class) && exp < ActiveModel::Serializer }
end

def event_name
'render.active_model_serializers'
end
end

private

def setup_serialization_subscriptions
@assert_serializer = AssertSerializer.new
@assert_serializer.subscribe
end
end
end
end
73 changes: 73 additions & 0 deletions test/active_model_serializers/test/serializer_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
require 'test_helper'
require 'rails-controller-testing' if Rails::VERSION::MAJOR >= 5
Copy link
Member

Choose a reason for hiding this comment

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

What's in here that needed 'rails-controller-testing'?

Copy link
Member Author

Choose a reason for hiding this comment

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

This test uses, this test was based on the original codebase of the test helper.

Copy link
Member

Choose a reason for hiding this comment

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

@maurogeorge that's kind of hilarious because it was removed from rails due to it being too intimate a test rails/rails#18950
We can remove that test and the dependency.

Do you have a link to the file you're basing this on? Would be nice to see.

Copy link
Member Author

Choose a reason for hiding this comment

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

@bf4 Yeah I know about the Rails decision removing this test helpers.
But I am not sure if we can remove this test, I think this was added to test the notification was not overwrite, based on the description of the test.

This is the last version of the file

I found.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, but we could subscribe to our own notifications here, right?

Copy link
Member

Choose a reason for hiding this comment

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

I can look at that in a followup PR if you'd prefer. I think that'd be nicer at this point :)

Copy link
Member Author

Choose a reason for hiding this comment

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

@bf4 I am ok with that, I think you can follow the same idea you did on this #1322 squash all these commits into a single one, create a new PR and close this one, and @mention me so I can follow up 😄


module ActiveModelSerializers
module Test
class SerializerTest < ActionController::TestCase
include ActiveModelSerializers::Test::Serializer

class MyController < ActionController::Base
def render_using_serializer
render json: Profile.new(name: 'Name 1', description: 'Description 1', comments: 'Comments 1')
end

def render_text
render text: 'ok'
end

def render_template
prepend_view_path './test/fixtures'
render template: 'template'
end
end

tests MyController

def test_supports_specifying_serializers_with_a_serializer_class
get :render_using_serializer
assert_serializer ProfileSerializer
end

def test_supports_specifying_serializers_with_a_regexp
get :render_using_serializer
assert_serializer(/\AProfile.+\Z/)
end

def test_supports_specifying_serializers_with_a_string
get :render_using_serializer
assert_serializer 'ProfileSerializer'
end

def test_supports_specifying_serializers_with_a_symbol
get :render_using_serializer
assert_serializer :profile_serializer
end

def test_supports_specifying_serializers_with_a_nil
get :render_text
assert_serializer nil
end

def test_raises_descriptive_error_message_when_serializer_was_not_rendered
get :render_using_serializer
e = assert_raise ActiveSupport::TestCase::Assertion do
assert_serializer 'PostSerializer'
end
assert_match 'expecting <"PostSerializer"> but rendering with <["ProfileSerializer"]>', e.message
end

def test_raises_argument_error_when_asserting_with_invalid_object
get :render_using_serializer
e = assert_raise ArgumentError do
assert_serializer Hash
end
assert_match 'assert_serializer only accepts a String, Symbol, Regexp, ActiveModel::Serializer, or nil', e.message
end

def test_does_not_overwrite_notification_subscriptions
get :render_template
assert_template 'template'
end
end
end
end
1 change: 1 addition & 0 deletions test/fixtures/template.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<p>Hello.</p>