Commit 97d18675 authored by Rafael Reggiani Manzo's avatar Rafael Reggiani Manzo

Merge branch 'rubocop' into 'master'

Add Rubocop

This MR:
- adds the rubocop gem
- adds the rubocop rake task
- adds the rubocop rake task to the default tasks
- fixes all current offenses

See merge request !32
parents 9d265deb ea7414f6
Pipeline #3718930 passed with stage
in 12 minutes and 32 seconds
......@@ -62,3 +62,14 @@ teaspoon:
- ./bin/setup
script:
- bundle exec rake teaspoon
rubocop:
stage: test
before_script:
- apt-get update -qq && apt-get install -y build-essential libpq-dev sqlite3 libsqlite3-dev
- gem install bundler --no-ri --no-rdoc
- mkdir -p tmp/gems
- bundle install --jobs $(nproc) "${FLAGS[@]}" --path=tmp/gems
- ./bin/setup
script:
- bundle exec rake rubocop
AllCops:
Exclude:
- 'bin/**/*'
- 'db/**/*'
- 'config/**/*'
- 'script/**/*'
- 'features/support/env.rb'
- 'vendor/**/*'
- 'tmp/**/*'
Rails:
Enabled: true
Style/AlignParameters:
EnforcedStyle: with_fixed_indentation
Style/CaseIndentation:
IndentWhenRelativeTo: end
Style/AsciiComments:
Enabled: false
Style/IndentHash:
Enabled: false
Style/CollectionMethods:
Enabled: true
PreferredMethods:
inject: 'inject'
Style/Documentation:
Enabled: false
Style/BlockDelimiters:
Exclude:
- spec/**/*_spec.rb
Style/BracesAroundHashParameters:
Exclude:
- spec/**/*_spec.rb
Style/GuardClause:
Enabled: false
Style/IfUnlessModifier:
Enabled: false
Style/SpaceInsideHashLiteralBraces:
Enabled: false
Style/Lambda:
Enabled: false
Style/RaiseArgs:
Enabled: false
Style/SignalException:
Enabled: false
Metrics/AbcSize:
Max: 20
Metrics/ClassLength:
Max: 100
Metrics/ModuleLength:
Max: 100
Metrics/LineLength:
Enabled: false
Metrics/MethodLength:
Max: 15
Style/SingleLineBlockParams:
Enabled: false
Lint/EndAlignment:
AlignWith: variable
Style/FormatString:
Enabled: false
Style/MultilineMethodCallIndentation:
EnforcedStyle: indented
Style/MultilineOperationIndentation:
EnforcedStyle: indented
Style/WordArray:
Enabled: false
Style/RedundantSelf:
Enabled: false
Style/AlignHash:
Enabled: true
EnforcedLastArgumentHashStyle: always_ignore
Style/TrivialAccessors:
AllowPredicates: true
Style/FrozenStringLiteralComment:
Enabled: false
Style/ConditionalAssignment:
Enabled: false
Style/RedundantBegin:
Enabled: false
Style/TrailingBlankLines:
EnforcedStyle: final_newline
Enabled: true
......@@ -78,12 +78,12 @@ group :development, :test do
# Call 'byebug' anywhere in the code to stop execution and get a debugger console
gem 'byebug', platform: :mri
#Unit test framework
# Unit test framework
# This beta is tested against Rails 5. Once it gets released use the stable version.
gem 'rspec-rails', '>= 3.5.0.beta1'
# Javascript test runner for Rails
gem "teaspoon-mocha"
gem 'teaspoon-mocha'
# Fixtures creation and management
gem 'factory_girl'
......@@ -91,6 +91,9 @@ group :development, :test do
# Test zencoder notifications
# PR address https://github.com/zencoder/zencoder-fetcher/pull/11
gem 'zencoder-fetcher', git: 'https://github.com/marcheing/zencoder-fetcher.git'
# Static code analyzer based on the ruby community guidelines: github.com/styleguide/ruby
gem 'rubocop', '~> 0.41.2', require: false
end
group :development do
......@@ -105,7 +108,7 @@ end
group :test do
# Acceptance tests framework
gem 'cucumber-rails', :require => false
gem 'cucumber-rails', require: false
# Database cleanup for each scenario
gem 'database_cleaner'
......@@ -117,7 +120,7 @@ group :test do
gem 'shoulda-matchers', '~> 3.1'
# Test coverage report
gem 'simplecov', :require => false
gem 'simplecov', require: false
end
# Windows does not include zoneinfo files, so bundle the tzinfo-data gem
......
......@@ -58,6 +58,7 @@ GEM
tzinfo (~> 1.1)
addressable (2.4.0)
arel (7.0.0)
ast (2.3.0)
aws-sdk (2.3.14)
aws-sdk-resources (= 2.3.14)
aws-sdk-core (2.3.14)
......@@ -187,12 +188,15 @@ GEM
cocaine (~> 0.5.5)
mime-types
mimemagic (~> 0.3.0)
parser (2.3.1.2)
ast (~> 2.2)
pkg-config (1.1.7)
poltergeist (1.9.0)
capybara (~> 2.1)
cliver (~> 0.3.1)
multi_json (~> 1.0)
websocket-driver (>= 0.2.0)
powerpack (0.1.1)
puma (3.4.0)
rack (2.0.1)
rack-test (0.6.3)
......@@ -223,6 +227,7 @@ GEM
method_source
rake (>= 0.8.7)
thor (>= 0.18.1, < 2.0)
rainbow (2.1.0)
rake (11.2.2)
rb-fsevent (0.9.7)
rb-inotify (0.9.7)
......@@ -247,6 +252,13 @@ GEM
rspec-mocks (= 3.5.0.beta4)
rspec-support (= 3.5.0.beta4)
rspec-support (3.5.0.beta4)
rubocop (0.41.2)
parser (>= 2.3.1.1, < 3.0)
powerpack (~> 0.1)
rainbow (>= 1.99.1, < 3.0)
ruby-progressbar (~> 1.7)
unicode-display_width (~> 1.0, >= 1.0.1)
ruby-progressbar (1.8.1)
sass (3.4.22)
sass-rails (5.0.4)
railties (>= 4.0.0, < 5.0)
......@@ -296,6 +308,7 @@ GEM
thread_safe (~> 0.1)
uglifier (3.0.0)
execjs (>= 0.3.0, < 3)
unicode-display_width (1.1.0)
warden (1.2.6)
rack (>= 1.0)
web-console (3.2.0)
......@@ -335,6 +348,7 @@ DEPENDENCIES
puma (~> 3.0)
rails (>= 5.0.0.rc2, < 5.1)
rspec-rails (>= 3.5.0.beta1)
rubocop (~> 0.41.2)
sass-rails (~> 5.0)
settingslogic
shoulda-matchers (~> 3.1)
......
......@@ -33,37 +33,42 @@ class ContentsController < ApplicationController
def zencoder_callback
@encoded_content = EncodedContent.find_by(job_id: params[:job][:id])
unless @encoded_content.nil?
@encoded_content.output_id = params[:outputs].first[:id]
@encoded_content.state = params[:outputs].first[:state]
@encoded_content.width = params[:outputs].first[:width]
@encoded_content.height = params[:outputs].first[:height]
@encoded_content.duration = params[:outputs].first[:duration_in_ms]
@encoded_content.file_size = params[:outputs].first[:file_size_in_bytes]
@encoded_content.video = URI.parse(params[:outputs].first[:url])
@encoded_content.thumbnail = URI.parse(params[:outputs].first[:thumbnails].first[:images].first[:url])
@encoded_content.save
update_encoded_content @encoded_content, params[:outputs].first
end
head :no_content
end
private
# Use callbacks to share common setup or constraints between actions.
def set_content
@content = Content.find(params[:id])
end
# Never trust parameters from the scary internet, only allow the white list through.
def content_params
params.require(:content).permit(:title, :user_id, :adult, :rating, :soundtrack, :view_count, :deleted, :zip_code, :director, :co_director, :team, :allow_comments, :video)
end
def update_encoded_content(encoded_content, parameters)
encoded_content.output_id = parameters[:id]
encoded_content.state = parameters[:state]
encoded_content.width = parameters[:width]
encoded_content.height = parameters[:height]
encoded_content.duration = parameters[:duration_in_ms]
encoded_content.file_size = parameters[:file_size_in_bytes]
encoded_content.video = URI.parse(parameters[:url])
encoded_content.thumbnail = URI.parse(parameters[:thumbnails].first[:images].first[:url])
encoded_content.save
end
def find_last_encoded_video
@encoded_video = @content.encoded_contents.last
redirect_to root_path, notice: I18n.t('video_has_not_finished_encoding') unless @encoded_video.state == 'finished'
end
# Use callbacks to share common setup or constraints between actions.
def set_content
@content = Content.find(params[:id])
end
def check_profile
redirect_to new_profile_path, notice: I18n.t('need_complete_profile_to_send_content') if current_user.profile.nil?
end
def check_profile
redirect_to new_profile_path, notice: I18n.t('need_complete_profile_to_send_content') if current_user.profile.nil?
end
# Never trust parameters from the scary internet, only allow the white list through.
def content_params
params.require(:content).permit(:title, :user_id, :adult, :rating, :soundtrack, :view_count, :deleted, :zip_code, :director, :co_director, :team, :allow_comments, :video)
end
def find_last_encoded_video
@encoded_video = @content.encoded_contents.last
redirect_to root_path, notice: I18n.t('video_has_not_finished_encoding') unless @encoded_video.state == 'finished'
end
end
......@@ -57,23 +57,24 @@ class ProfilesController < ApplicationController
end
private
# Use callbacks to share common setup or constraints between actions.
def set_profile
@profile = Profile.find(params[:id])
end
# Never trust parameters from the scary internet, only allow the white list through.
def profile_params
complete_params = params.require(:profile).permit(:first_name, :last_name, :gender, :state, :city, :country, :school, :responsible_document, :responsible_name, :receive_mass_email, :phone, :user_id, :address, :zip_code, :occupation, :school_type)
complete_params[:birthdate] = Date.new params[:profile][:year].to_i, params[:profile][:month].to_i, params[:profile][:day].to_i
complete_params
end
# Use callbacks to share common setup or constraints between actions.
def set_profile
@profile = Profile.find(params[:id])
end
def set_carmen_locale
Carmen.i18n_backend.locale = I18n.locale.to_s.split('-').first
end
# Never trust parameters from the scary internet, only allow the white list through.
def profile_params
complete_params = params.require(:profile).permit(:first_name, :last_name, :gender, :state, :city, :country, :school, :responsible_document, :responsible_name, :receive_mass_email, :phone, :user_id, :address, :zip_code, :occupation, :school_type)
complete_params[:birthdate] = Date.new params[:profile][:year].to_i, params[:profile][:month].to_i, params[:profile][:day].to_i
complete_params
end
def states_params
params.require(:country)
end
def set_carmen_locale
Carmen.i18n_backend.locale = I18n.locale.to_s.split('-').first
end
def states_params
params.require(:country)
end
end
......@@ -4,7 +4,7 @@ class Content < ApplicationRecord
s3_credentials: AWSSettings.to_h,
s3_region: AWSSettings.aws_region
validates_attachment :video, presence: true
validates_attachment_content_type :video, content_type: /\Avideo\/.*\Z/
validates_attachment_content_type :video, content_type: %r{\Avideo\/.*\Z}
validates :terms_of_service, acceptance: true
......@@ -22,4 +22,3 @@ class Content < ApplicationRecord
RequestZencoderJob.perform_later(self)
end
end
......@@ -9,7 +9,7 @@ class EncodedContent < ApplicationRecord
s3_credentials: AWSSettings.to_h,
s3_region: AWSSettings.aws_region
validates_attachment_content_type :video, content_type: /\Avideo\/.*\Z/
validates_attachment_content_type :video, content_type: %r{\Avideo\/.*\Z}
validates_attachment_content_type :thumbnail, content_type: /\Aimage/
validates :content_id, :job_id, presence: true
......
......@@ -3,7 +3,7 @@ class Profile < ApplicationRecord
validates :terms_of_service, acceptance: true
validates :first_name, :last_name, :country, :state, :city,
:zip_code, :birthdate, :gender, :occupation, presence: true
:zip_code, :birthdate, :gender, :occupation, presence: true
validates :responsible_name, :responsible_document, presence: true, if: :underaged?
def adult?
......
......@@ -2,8 +2,8 @@ class User < ApplicationRecord
# Include default devise modules. Others available are:
# :confirmable, :lockable, :timeoutable and :omniauthable
devise :database_authenticatable, :registerable,
:recoverable, :rememberable, :trackable, :validatable,
:confirmable, :lockable, :timeoutable
:recoverable, :rememberable, :trackable, :validatable,
:confirmable, :lockable, :timeoutable
validates :email, confirmation: true
......
......@@ -27,7 +27,7 @@ When(/^I fill the director field with my name$/) do
end
When(/^I mark the specific location checkbox$/) do
step "I check the \"video-has-location\" box"
step 'I check the "video-has-location" box'
end
When(/^I fill the zip code field with my zip code$/) do
......
......@@ -7,7 +7,7 @@ Given(/^I should see "(.*?)"$/) do |content|
end
When(/^I take a picture of the page$/) do
page.save_screenshot("/tmp/picture.png")
page.save_screenshot('/tmp/picture.png')
end
When(/^I click on the "(.*?)" link$/) do |link|
......
......@@ -33,4 +33,3 @@ end
Then(/^I should be able to see the share button$/) do
expect(has_css?('a[id^="vjs-share-button"]')).to eq true
end
......@@ -11,7 +11,7 @@ Given(/^I select "(.*?)" as my birthdate's (.*?)$/) do |value, field|
end
Given(/^I mark the terms of service checkbox$/) do
step "I check the \"user-terms\" box"
step 'I check the "user-terms" box'
end
When(/^I click on the button to send the profile information$/) do
......@@ -57,7 +57,7 @@ Given(/^I try to see a nonexistent profile$/) do
end
Then(/^I should see the error message for occupation can't be blank$/) do
step "I should see \"#{I18n.t 'errors.format', {attribute: Profile.human_attribute_name('occupation'), message: I18n.t('errors.messages.blank')}}\""
step "I should see \"#{I18n.t 'errors.format', attribute: Profile.human_attribute_name('occupation'), message: I18n.t('errors.messages.blank')}\""
end
Then(/^I should see the not found page$/) do
......
require 'English'
Given(/^the user has a complete profile$/) do
@profile = create(:profile, user_id: @user.id)
end
......@@ -25,7 +27,7 @@ end
Then(/^I should receive a successful notification$/) do
`zencoder_fetcher -u #{ZencoderSettings.notifications.url} #{ZencoderSettings.api_key} -n 1 -x`
expect($?.success?).to eq true
expect($CHILD_STATUS.success?).to eq true
end
Then(/^the encoded content should have been updated$/) do
......
# Run all acceptance tests on the default language
Before do |scenario|
if defined?(page) && ! page.driver.nil?
Before do |_scenario|
if defined?(page) && !page.driver.nil?
header_method = [:add_header, :header].find(&page.driver.method(:respond_to?))
page.driver.send(header_method, 'Accept-Language', I18n.default_locale) if header_method
end
......
......@@ -4,73 +4,72 @@
# instead of editing this one. Cucumber will automatically load all features/**/*.rb
# files.
unless ARGV.any? { |a| a =~ /^gems/ } # Don't load anything when running the gems:* tasks
unless ARGV.any? {|a| a =~ /^gems/} # Don't load anything when running the gems:* tasks
vendored_cucumber_bin = Dir["#{Rails.root}/vendor/{gems,plugins}/cucumber*/bin/cucumber"].first
$LOAD_PATH.unshift(File.dirname(vendored_cucumber_bin) + '/../lib') unless vendored_cucumber_bin.nil?
vendored_cucumber_bin = Dir["#{Rails.root}/vendor/{gems,plugins}/cucumber*/bin/cucumber"].first
$LOAD_PATH.unshift(File.dirname(vendored_cucumber_bin) + '/../lib') unless vendored_cucumber_bin.nil?
begin
require 'cucumber/rake/task'
begin
require 'cucumber/rake/task'
namespace :cucumber do
Cucumber::Rake::Task.new({:ok => 'test:prepare'}, 'Run features that should pass') do |t|
t.binary = vendored_cucumber_bin # If nil, the gem's binary is used.
t.fork = true # You may get faster startup if you set this to false
t.profile = 'default'
end
namespace :cucumber do
Cucumber::Rake::Task.new({ok: 'test:prepare'}, 'Run features that should pass') do |t|
t.binary = vendored_cucumber_bin # If nil, the gem's binary is used.
t.fork = true # You may get faster startup if you set this to false
t.profile = 'default'
end
Cucumber::Rake::Task.new({:wip => 'test:prepare'}, 'Run features that are being worked on') do |t|
t.binary = vendored_cucumber_bin
t.fork = true # You may get faster startup if you set this to false
t.profile = 'wip'
end
Cucumber::Rake::Task.new({wip: 'test:prepare'}, 'Run features that are being worked on') do |t|
t.binary = vendored_cucumber_bin
t.fork = true # You may get faster startup if you set this to false
t.profile = 'wip'
end
Cucumber::Rake::Task.new({:rerun => 'test:prepare'}, 'Record failing features and run only them if any exist') do |t|
t.binary = vendored_cucumber_bin
t.fork = true # You may get faster startup if you set this to false
t.profile = 'rerun'
end
Cucumber::Rake::Task.new({rerun: 'test:prepare'}, 'Record failing features and run only them if any exist') do |t|
t.binary = vendored_cucumber_bin
t.fork = true # You may get faster startup if you set this to false
t.profile = 'rerun'
end
desc 'Run all features'
task :all => [:ok, :wip]
desc 'Run all features'
task all: [:ok, :wip]
task :statsetup do
require 'rails/code_statistics'
::STATS_DIRECTORIES << %w(Cucumber\ features features) if File.exist?('features')
::CodeStatistics::TEST_TYPES << "Cucumber features" if File.exist?('features')
end
task :statsetup do
require 'rails/code_statistics'
::STATS_DIRECTORIES << %w(Cucumber\ features features) if File.exist?('features')
::CodeStatistics::TEST_TYPES << 'Cucumber features' if File.exist?('features')
end
task :annotations_setup do
Rails.application.configure do
if config.respond_to?(:annotations)
config.annotations.directories << 'features'
config.annotations.register_extensions('feature') { |tag| /#\s*(#{tag}):?\s*(.*)$/ }
task :annotations_setup do
Rails.application.configure do
if config.respond_to?(:annotations)
config.annotations.directories << 'features'
config.annotations.register_extensions('feature') { |tag| /#\s*(#{tag}):?\s*(.*)$/ }
end
end
end
end
end
desc 'Alias for cucumber:ok'
task :cucumber => 'cucumber:ok'
desc 'Alias for cucumber:ok'
task cucumber: 'cucumber:ok'
task :default => :cucumber
task default: :cucumber
task :features => :cucumber do
STDERR.puts "*** The 'features' task is deprecated. See rake -T cucumber ***"
end
task features: :cucumber do
STDERR.puts "*** The 'features' task is deprecated. See rake -T cucumber ***"
end
# In case we don't have the generic Rails test:prepare hook, append a no-op task that we can depend upon.
task 'test:prepare' do
end
# In case we don't have the generic Rails test:prepare hook, append a no-op task that we can depend upon.
task 'test:prepare' do
end
task :stats => 'cucumber:statsetup'
task stats: 'cucumber:statsetup'
task :notes => 'cucumber:annotations_setup'
rescue LoadError
desc 'cucumber rake task not available (cucumber not installed)'
task :cucumber do
abort 'Cucumber rake task is not available. Be sure to install cucumber as a gem or plugin'
task notes: 'cucumber:annotations_setup'
rescue LoadError
desc 'cucumber rake task not available (cucumber not installed)'
task :cucumber do
abort 'Cucumber rake task is not available. Be sure to install cucumber as a gem or plugin'
end
end
end
end
require 'rubocop/rake_task'
RuboCop::RakeTask.new
task default: 'rubocop'
task :default => 'teaspoon'
task default: 'teaspoon'
......@@ -18,7 +18,7 @@ require 'rails_helper'
# Message expectations are only used when there is no simpler way to specify
# that an instance is receiving a specific message.
RSpec.describe ContentsController, type: :controller do
describe ContentsController do
include Devise::TestHelpers
let(:valid_request_parameters) {
......@@ -128,9 +128,11 @@ RSpec.describe ContentsController, type: :controller do
describe 'POST #zencoder_callback' do
let!(:job_id) { '1' }
let!(:thumb_output) { [ images: [ url: 'http://test.com' ] ] }
let!(:output) { { id: '2', state: 'finished', width: '640', height: '320', duration_in_ms: '5000',
file_size_in_bytes: '1024', url: 'http://test.com', thumbnails: thumb_output } }
let!(:thumb_output) { [images: [url: 'http://test.com']] }
let!(:output) {
{ id: '2', state: 'finished', width: '640', height: '320', duration_in_ms: '5000',
file_size_in_bytes: '1024', url: 'http://test.com', thumbnails: thumb_output }
}
let!(:outputs) { [output] }
let!(:video_file_mock) { instance_double('video_file') }
let!(:thumb_file_mock) { instance_double('thumb_file') }
......
require 'rails_helper'
describe HomeController, :type => :controller do
describe HomeController do
context 'actions' do
context 'index' do
describe 'Rendering' do
......
......@@ -18,7 +18,7 @@ require 'rails_helper'
# Message expectations are only used when there is no simpler way to specify
# that an instance is receiving a specific message.
RSpec.describe ProfilesController, type: :controller do
describe ProfilesController do
include Devise::TestHelpers
let(:valid_request_parameters) do
......@@ -65,21 +65,21 @@ RSpec.describe ProfilesController, type: :controller do
end
end
describe "POST #create" do
context "with valid params" do
it "creates a new Profile" do
describe 'POST #create' do
context 'with valid params' do
it 'creates a new Profile' do
expect {
post :create, params: { profile: valid_request_parameters }
}.to change(Profile, :count).by(1)
end
it "redirects to the created profile" do
it 'redirects to the created profile' do
post :create, params: { profile: valid_request_parameters }
expect(response).to redirect_to(root_path)
end
end
context "with invalid params" do
context 'with invalid params' do
before do
expect_any_instance_of(Profile).to receive(:save).and_return false
post :create, params: { profile: valid_request_parameters }
......@@ -89,31 +89,31 @@ RSpec.describe ProfilesController, type: :controller do
end
end
describe "PUT #update" do
describe 'PUT #update' do
let(:profile) { build(:profile) }
before do
expect(Profile).to receive(:find).with(profile.id.to_s).and_return profile
end
context "with valid params" do
context 'with valid params' do
let(:new_attributes) {
skip("Add a hash of attributes valid for your model")
skip('Add a hash of attributes valid for your model')
}
it "updates the requested profile" do
it 'updates the requested profile' do
put :update, params: {id: profile.to_param, profile: new_attributes}
profile.reload
skip("Add assertions for updated state")
skip('Add assertions for updated state')
end
it "redirects to the profile" do
it 'redirects to the profile' do
put :update, params: {id: profile.to_param, profile: valid_request_parameters }
expect(response).to redirect_to(profile)
end
end
context "with invalid params" do
context 'with invalid params' do
before do
expect_any_instance_of(Profile).to receive(:update).and_return false
put :update, params: {id: profile.to_param, profile: valid_request_parameters }
......@@ -123,19 +123,19 @@ RSpec.describe ProfilesController, type: :controller do
end
end
describe "DELETE #destroy" do
describe 'DELETE #destroy' do
let(:profile) { build(:profile) }
before do
expect(Profile).to receive(:find).with(profile.id.to_s).and_return profile
end
it "destroys the requested profile" do
it 'destroys the requested profile' do
expect(profile).to receive(:destroy)
delete :destroy, params: { id: profile.id }
end
it "redirects to the profiles list" do
it 'redirects to the profiles list' do