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

add rspec messages #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
21 changes: 13 additions & 8 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ gem 'jquery-rails'
# Use ActiveModel has_secure_password
# gem 'bcrypt', '~> 3.1.7'

# a library for generating fake data.
gem 'faker'
Copy link
Member

Choose a reason for hiding this comment

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

It is desirable to be in the test group.

Copy link
Author

Choose a reason for hiding this comment

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

I would like use gem faker to generate data at random in db/seeds.rb.

Copy link
Member

Choose a reason for hiding this comment

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

If so, should you limit it with development and test? We do not need it for staging or production.


# Use Capistrano for deployment
# gem 'capistrano-rails', group: :development

Expand All @@ -44,18 +47,20 @@ group :development, :test do

# rspec
gem 'rspec-rails', '~> 3.5'
gem 'shoulda-matchers'

# RSpec::JsonExpectations
gem 'rspec-json_expectations'
end

group :test do
# for build strategies
gem 'factory_girl_rails'

# database cleaner
gem 'database_rewinder'
# a library for setting up Ruby objects as test data.
gem 'factory_girl_rails', '~> 4.0'
Copy link
Member

Choose a reason for hiding this comment

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

Why did you add version designation?

Copy link
Author

Choose a reason for hiding this comment

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

Habit, I wouldn't like to be problem with oldest versions.

Copy link
Member

Choose a reason for hiding this comment

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

I specified the lowest version in Gemfile.lock. I am worried about inhibiting version up by writing this.

# shoulda-matcher provides Rspec and Minitest compatible one liners that test common Rails functionality.
gem 'shoulda-matchers', '~> 3.1'
# strategies for cleaning database in Ruby.
gem 'database_cleaner'
Copy link
Member

Choose a reason for hiding this comment

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

There is no need to change from database_rewinder.

# an IRB alternative and runtime developer console.
gem 'pry'
Copy link
Member

Choose a reason for hiding this comment

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

You should add it to development and test if necessary.

end

group :development do
Expand All @@ -70,7 +75,7 @@ end
# Windows does not include zoneinfo files, so bundle the tzinfo-data gem
gem 'tzinfo-data', platforms: [:mingw, :mswin, :x64_mingw, :jruby]

# json genarator
gem 'active_model_serializers'
# ActiveModel::Serializer implementation and Rails hooks.
gem 'active_model_serializers', '~> 0.10.0'
Copy link
Member

Choose a reason for hiding this comment

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

Why did you add version designation?

Copy link
Author

Choose a reason for hiding this comment

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

Habit, I wouldn't like to be problem with oldest versions.

Copy link
Member

Choose a reason for hiding this comment

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

I specified the lowest version in Gemfile.lock. I am worried about inhibiting version up by writing this.


gem 'foreman'
22 changes: 16 additions & 6 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,9 @@ GEM
byebug (9.0.6)
case_transform (0.2)
activesupport
coderay (1.1.1)
concurrent-ruby (1.0.4)
database_rewinder (0.8.0)
database_cleaner (1.5.3)
debug_inspector (0.0.2)
diff-lcs (1.3)
erubis (2.7.0)
Expand All @@ -59,6 +60,8 @@ GEM
factory_girl_rails (4.8.0)
factory_girl (~> 4.8.0)
railties (>= 3.0.0)
faker (1.7.3)
i18n (~> 0.5)
ffi (1.9.17)
foreman (0.83.0)
thor (~> 0.19.1)
Expand Down Expand Up @@ -90,6 +93,10 @@ GEM
nio4r (1.2.1)
nokogiri (1.7.0.1)
mini_portile2 (~> 2.1.0)
pry (0.10.4)
coderay (~> 1.1.0)
method_source (~> 0.8.1)
slop (~> 3.4)
puma (3.7.0)
rack (2.0.1)
rack-cors (0.4.1)
Expand Down Expand Up @@ -149,6 +156,7 @@ GEM
tilt (>= 1.1, < 3)
shoulda-matchers (3.1.1)
activesupport (>= 4.0.0)
slop (3.6.0)
spring (2.0.1)
activesupport (>= 4.2)
spring-watcher-listen (2.0.1)
Expand Down Expand Up @@ -182,20 +190,22 @@ PLATFORMS
ruby

DEPENDENCIES
active_model_serializers
active_model_serializers (~> 0.10.0)
byebug
database_rewinder
factory_girl_rails
database_cleaner
factory_girl_rails (~> 4.0)
faker
foreman
jquery-rails
listen (~> 3.0.5)
pry
puma (~> 3.0)
rack-cors
rails (~> 5.0.1)
rspec-json_expectations
rspec-rails (~> 3.5)
sass-rails (~> 5.0)
shoulda-matchers
shoulda-matchers (~> 3.1)
spring
spring-watcher-listen (~> 2.0.0)
sqlite3
Expand All @@ -204,4 +214,4 @@ DEPENDENCIES
web-console (>= 3.3.0)

BUNDLED WITH
1.14.3
1.14.6
2 changes: 2 additions & 0 deletions app/controllers/api/api_controller.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
module Api
class ApiController < ActionController::API
include Response
include ExceptionHandler
end
end
34 changes: 13 additions & 21 deletions app/controllers/api/messages_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,41 +2,33 @@ module Api
class MessagesController < ApiController
before_action :set_message, only: [:show, :update, :destroy]

# GET /messages
# GET / messages
def index
@messages = Message.order(created_at: :asc).all

render json: @messages
json_response(@messages)
end

# GET /messages/1
# GET / messages / :id
def show
render json: @message
json_response(@message)
end

# POST /messages
# POST / messages
def create
@message = Message.new(message_params)

if @message.save
render json: @message, status: :created, location: [:admin, @message]
else
render json: @message.errors, status: :unprocessable_entity
end
@message = Message.create!(message_params)
json_response(@message, :created)
end

# PATCH/PUT /messages/1
# PUT / messages / :id
Copy link
Member

Choose a reason for hiding this comment

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

It should be patch. Please check bin/rake routes.

def update
if @message.update(message_params)
render json: @message
else
render json: @message.errors, status: :unprocessable_entity
end
@message.update(message_params)
head :no_content
Copy link
Member

Choose a reason for hiding this comment

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

What is the reason for changing?

Copy link
Author

Choose a reason for hiding this comment

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

This is the expected behavior in case of a DELETE and UPDATE request, because if you are actually not sending any content, it is best to mention it with a :no_content.

end

# DELETE /messages/1
# DELETE / messages / :id
def destroy
@message.destroy
head :no_content
Copy link
Member

Choose a reason for hiding this comment

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

Is this necessary?

Copy link
Author

Choose a reason for hiding this comment

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

This is the expected behavior in case of a DELETE and UPDATE request, because if you are actually not sending any content, it is best to mention it with a :no_content.

end

private
Expand All @@ -47,7 +39,7 @@ def set_message

# Only allow a trusted parameter "white list" through.
def message_params
params.require(:message).permit(:text)
params.permit(:text)
Copy link
Member

Choose a reason for hiding this comment

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

What is the reason for changing?

end
end
end
13 changes: 13 additions & 0 deletions app/controllers/concerns/exception_handler.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
module ExceptionHandler
extend ActiveSupport::Concern

included do
rescue_from ActiveRecord::RecordNotFound do |e|
json_response({ message: e.message }, :not_found)
Copy link
Member

Choose a reason for hiding this comment

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

Since modules are not loosely coupled, it is not recommended to do such a construction.

Copy link
Author

Choose a reason for hiding this comment

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

Since browsers only read 4xx & 5xx error codes, all Rails exceptions have to be inferred. Thus, ExceptionHandler simply has to manage how the 4xx / 5xx errors are passed to the browser and it provides the more graceful included method.

end

rescue_from ActiveRecord::RecordInvalid do |e|
json_response({ message: e.message }, :unprocessable_entity)
end
end
end
5 changes: 5 additions & 0 deletions app/controllers/concerns/response.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module Response
def json_response(object, status = :ok)
render json: object, status: status
end
end
3 changes: 3 additions & 0 deletions app/models/message.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
class Message < ApplicationRecord
# model association
has_many :reactions, dependent: :destroy
# validation
validates_presence_of :text
end
5 changes: 5 additions & 0 deletions spec/factories/messages.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
FactoryGirl.define do
factory :message do
text { Faker::Lorem.sentences }
end
end
7 changes: 6 additions & 1 deletion spec/models/message_spec.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
require 'rails_helper'

RSpec.describe Message, type: :model do
#pending "add some examples to (or delete) #{__FILE__}"
# association test
# ensure Message model has a 1:m relationship with the Reaction model
it { should have_many(:reactions).dependent(:destroy) }
# validation tests
# ensure columns text is present before saving
it { should validate_presence_of(:text) }
end
30 changes: 28 additions & 2 deletions spec/rails_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,16 @@
abort("The Rails environment is running in production mode!") if Rails.env.production?
require 'spec_helper'
require 'rspec/rails'
# Add additional requires below this line. Rails is not loaded until this point!
# require database cleaner at the top level
require 'database_cleaner'

# configure shoulda matchers to use rspec as the test framework and full matcher libraries for rails
Shoulda::Matchers.configure do |config|
config.integrate do |with|
with.test_framework :rspec
with.library :rails
end
end

# Requires supporting ruby files with custom matchers and macros, etc, in
# spec/support/ and its subdirectories. Files matching `spec/**/*_spec.rb` are
Expand All @@ -20,7 +29,7 @@
# directory. Alternatively, in the individual `*_spec.rb` files, manually
# require only the support files necessary.
#
# Dir[Rails.root.join('spec/support/**/*.rb')].each { |f| require f }
Dir[Rails.root.join('spec/support/**/*.rb')].each { |f| require f }

# Checks for pending migration and applies them before tests are run.
# If you are not using ActiveRecord, you can remove this line.
Expand All @@ -30,11 +39,28 @@
# Remove this line if you're not using ActiveRecord or ActiveRecord fixtures
config.fixture_path = "#{::Rails.root}/spec/fixtures"

config.include RequestSpecHelper, type: :request
# If you're not using ActiveRecord, or you'd prefer not to run each of your
# examples within a transaction, remove the following line or assign false
# instead of true.
config.use_transactional_fixtures = true

# add `FactoryGirl` methods
config.include FactoryGirl::Syntax::Methods

# start by truncating all the tables but then use the faster transaction strategy the rest of the time.
config.before(:suite) do
DatabaseCleaner.clean_with(:truncation)
DatabaseCleaner.strategy = :transaction
end

# start the transaction strategy as examples are run
config.around(:each) do |example|
DatabaseCleaner.cleaning do
example.run
end
end

# RSpec Rails can automatically mix in different behaviours to your tests
# based on their file location, for example enabling you to call `get` and
# `post` in specs under `spec/controllers`.
Expand Down
103 changes: 99 additions & 4 deletions spec/requests/messages_spec.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,105 @@
require 'rails_helper'

RSpec.describe "Messages", type: :request do
describe "GET /messages" do
it "works! (now write some real specs)" do
get api_messages_path
RSpec.describe "Messages API", type: :request do
# initialize tests data
let!(:messages) { create_list(:message, 10) }
let(:message_id) { messages.first.id }

# test suite for GET / messages
describe "GET / messages" do
before { get "/api/messages" }

it "returns status code 200" do
expect(response).to have_http_status(200)
end

it "returns messages" do
expect(json).not_to be_empty
expect(json.size).to eq(10)
end
end

# test suite for GET / messages / :id
describe "GET / messages / :id" do
before { get "/api/messages/#{message_id}" }

context "when the record exists" do
it "returns status code 200" do
expect(response).to have_http_status(200)
end

it "returns the message" do
expect(json).not_to be_empty
expect(json['id']).to eq(message_id)
end
end

context "when the record does not exist" do
let(:message_id) { 100 }

it "returns status code 404" do
expect(response).to have_http_status(404)
end

it "returns a not found message" do
expect(response.body).to match(/Couldn't find Message/)
end
end
end

# test suite for POST / messages
describe "POST / messages" do
let(:valid_attributes) { { text: "Text" } }
let(:invalid_attributes) { {} }

context "when the request is valid" do
before { post "/api/messages", params: valid_attributes }

it "returns status code 201" do
expect(response).to have_http_status(201)
end

it "creates a message" do
expect(json['text']).to eq("Text")
end
end

describe "when the request is invalid" do
before { post "/api/messages", params: invalid_attributes }

it "returns status code 422" do
expect(response).to have_http_status(422)
end

it "returns a validation failure message" do
expect(response.body).to match(/Validation failed: Text can't be blank/)
end
end
end

# test suite for PUT / messages / :id
describe "PUT / messages / :id" do
let(:valid_attributes) { { text: "Text" } }

context "when the record exists" do
before { put "/api/messages/#{message_id}", params: valid_attributes }

it "returns status code 204" do
expect(response).to have_http_status(204)
end

it "updates the record" do
expect(response.body).to be_empty
end
end
end

# test suite for DELETE / messages / :id
describe "DELETE / messages / :id" do
before { delete "/api/messages/#{message_id}" }

it "returns status code 204" do
expect(response).to have_http_status(204)
end
end
end
Loading