From 81c76fe375d9342e5a436db05c8e25305c650e8d Mon Sep 17 00:00:00 2001 From: Samy KACIMI Date: Wed, 5 Apr 2017 00:29:56 +0200 Subject: [PATCH 1/6] add more tests to models --- Gemfile | 1 + Gemfile.lock | 3 + app/models/block.rb | 5 +- app/models/follow.rb | 9 ++- app/models/follow_request.rb | 5 +- app/models/mention.rb | 5 +- config/database.yml | 4 ++ spec/fabricators/account_fabricator.rb | 2 +- spec/fabricators/block_fabricator.rb | 3 +- spec/fabricators/follow_fabricator.rb | 3 +- spec/fabricators/follow_request_fabricator.rb | 3 +- spec/fabricators/mention_fabricator.rb | 4 ++ spec/fabricators/user_fabricator.rb | 2 +- spec/models/account_spec.rb | 69 +++++++++++++++++++ spec/models/block_spec.rb | 17 +++++ spec/models/domain_block_spec.rb | 18 +++++ spec/models/follow_request_spec.rb | 19 +++++ spec/models/follow_spec.rb | 19 +++++ spec/models/mention_spec.rb | 17 +++++ spec/models/user_spec.rb | 44 ++++++++++++ spec/rails_helper.rb | 2 + .../model/model_have_error_on_field.rb | 15 ++++ 22 files changed, 252 insertions(+), 17 deletions(-) create mode 100644 spec/fabricators/mention_fabricator.rb create mode 100644 spec/support/matchers/model/model_have_error_on_field.rb diff --git a/Gemfile b/Gemfile index 4c6314763a..87ea77735c 100644 --- a/Gemfile +++ b/Gemfile @@ -70,6 +70,7 @@ group :test do gem 'simplecov', require: false gem 'webmock' gem 'rspec-sidekiq' + gem 'faker' end group :development do diff --git a/Gemfile.lock b/Gemfile.lock index 26c7b9962e..a774a89bab 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -149,6 +149,8 @@ GEM erubis (2.7.0) execjs (2.7.0) fabrication (2.15.2) + faker (1.6.6) + i18n (~> 0.5) fast_blank (1.0.0) font-awesome-rails (4.6.3.1) railties (>= 3.2, < 5.1) @@ -470,6 +472,7 @@ DEPENDENCIES doorkeeper dotenv-rails fabrication + faker fast_blank font-awesome-rails fuubar diff --git a/app/models/block.rb b/app/models/block.rb index 9c55703c97..ae456a6b6b 100644 --- a/app/models/block.rb +++ b/app/models/block.rb @@ -3,9 +3,8 @@ class Block < ApplicationRecord include Paginable - belongs_to :account - belongs_to :target_account, class_name: 'Account' + belongs_to :account, required: true + belongs_to :target_account, class_name: 'Account', required: true - validates :account, :target_account, presence: true validates :account_id, uniqueness: { scope: :target_account_id } end diff --git a/app/models/follow.rb b/app/models/follow.rb index 8bfe8b2f6c..fd7325f059 100644 --- a/app/models/follow.rb +++ b/app/models/follow.rb @@ -3,11 +3,14 @@ class Follow < ApplicationRecord include Paginable - belongs_to :account, counter_cache: :following_count - belongs_to :target_account, class_name: 'Account', counter_cache: :followers_count + belongs_to :account, counter_cache: :following_count, required: true + + belongs_to :target_account, + class_name: 'Account', + counter_cache: :followers_count, + required: true has_one :notification, as: :activity, dependent: :destroy - validates :account, :target_account, presence: true validates :account_id, uniqueness: { scope: :target_account_id } end diff --git a/app/models/follow_request.rb b/app/models/follow_request.rb index 4224ab15d9..20e1332ddd 100644 --- a/app/models/follow_request.rb +++ b/app/models/follow_request.rb @@ -3,12 +3,11 @@ class FollowRequest < ApplicationRecord include Paginable - belongs_to :account - belongs_to :target_account, class_name: 'Account' + belongs_to :account, required: true + belongs_to :target_account, class_name: 'Account', required: true has_one :notification, as: :activity, dependent: :destroy - validates :account, :target_account, presence: true validates :account_id, uniqueness: { scope: :target_account_id } def authorize! diff --git a/app/models/mention.rb b/app/models/mention.rb index 10a9cb1cd1..03e76fcc42 100644 --- a/app/models/mention.rb +++ b/app/models/mention.rb @@ -1,11 +1,10 @@ # frozen_string_literal: true class Mention < ApplicationRecord - belongs_to :account, inverse_of: :mentions - belongs_to :status + belongs_to :account, inverse_of: :mentions, required: true + belongs_to :status, required: true has_one :notification, as: :activity, dependent: :destroy - validates :account, :status, presence: true validates :account, uniqueness: { scope: :status } end diff --git a/config/database.yml b/config/database.yml index 5ec342f939..3901134806 100644 --- a/config/database.yml +++ b/config/database.yml @@ -3,6 +3,10 @@ default: &default pool: <%= ENV["DB_POOL"] || ENV['MAX_THREADS'] || 5 %> timeout: 5000 encoding: unicode + host: localhost + username: samy + password: tardis + port: 32769 development: <<: *default diff --git a/spec/fabricators/account_fabricator.rb b/spec/fabricators/account_fabricator.rb index 3a7c00bf55..567de05f4b 100644 --- a/spec/fabricators/account_fabricator.rb +++ b/spec/fabricators/account_fabricator.rb @@ -1,3 +1,3 @@ Fabricator(:account) do - username "alice" + username { Faker::Internet.user_name(nil, %w(_)) } end diff --git a/spec/fabricators/block_fabricator.rb b/spec/fabricators/block_fabricator.rb index 9a5a6808f1..379931ba65 100644 --- a/spec/fabricators/block_fabricator.rb +++ b/spec/fabricators/block_fabricator.rb @@ -1,3 +1,4 @@ Fabricator(:block) do - + account + target_account { Fabricate(:account) } end diff --git a/spec/fabricators/follow_fabricator.rb b/spec/fabricators/follow_fabricator.rb index 9d9d06f125..9b25dc547b 100644 --- a/spec/fabricators/follow_fabricator.rb +++ b/spec/fabricators/follow_fabricator.rb @@ -1,3 +1,4 @@ Fabricator(:follow) do - + account + target_account { Fabricate(:account) } end diff --git a/spec/fabricators/follow_request_fabricator.rb b/spec/fabricators/follow_request_fabricator.rb index 9c3733cef8..78a057919d 100644 --- a/spec/fabricators/follow_request_fabricator.rb +++ b/spec/fabricators/follow_request_fabricator.rb @@ -1,3 +1,4 @@ Fabricator(:follow_request) do - + account + target_account { Fabricate(:account) } end diff --git a/spec/fabricators/mention_fabricator.rb b/spec/fabricators/mention_fabricator.rb new file mode 100644 index 0000000000..cb5fe4299a --- /dev/null +++ b/spec/fabricators/mention_fabricator.rb @@ -0,0 +1,4 @@ +Fabricator(:mention) do + account + status +end diff --git a/spec/fabricators/user_fabricator.rb b/spec/fabricators/user_fabricator.rb index c08559137f..16b3b1f6f6 100644 --- a/spec/fabricators/user_fabricator.rb +++ b/spec/fabricators/user_fabricator.rb @@ -1,6 +1,6 @@ Fabricator(:user) do account - email "alice@example.com" + email { Faker::Internet.email } password "123456789" confirmed_at { Time.now } end diff --git a/spec/models/account_spec.rb b/spec/models/account_spec.rb index 91c8d75cf4..fbc9a7d407 100644 --- a/spec/models/account_spec.rb +++ b/spec/models/account_spec.rb @@ -209,4 +209,73 @@ RSpec.describe Account, type: :model do expect(subject.match('Check this out https://medium.com/@alice/some-article#.abcdef123')).to be_nil end end + + describe 'validations' do + it 'has a valid fabricator' do + account = Fabricate.build(:account) + account.valid? + expect(account).to be_valid + end + + it 'is invalid without a username' do + account = Fabricate.build(:account, username: nil) + account.valid? + expect(account).to model_have_error_on_field(:username) + end + + it 'is invalid is the username already exists' do + account_1 = Fabricate(:account, username: 'the_doctor') + account_2 = Fabricate.build(:account, username: 'the_doctor') + account_2.valid? + expect(account_2).to model_have_error_on_field(:username) + end + + context 'when is local' do + it 'is invalid if the username doesn\'t only contains letters, numbers and underscores' do + account = Fabricate.build(:account, username: 'the-doctor') + account.valid? + expect(account).to model_have_error_on_field(:username) + end + + it 'is invalid if the username is longer then 30 characters' do + account = Fabricate.build(:account, username: Faker::Lorem.characters(31)) + account.valid? + expect(account).to model_have_error_on_field(:username) + end + end + end + + describe 'scopes' do + describe 'remote' do + it 'returns an array of accounts who have a domain' do + account_1 = Fabricate(:account, domain: nil) + account_2 = Fabricate(:account, domain: 'example.com') + expect(Account.remote).to match_array([account_2]) + end + end + + describe 'local' do + it 'returns an array of accounts who do not have a domain' do + account_1 = Fabricate(:account, domain: nil) + account_2 = Fabricate(:account, domain: 'example.com') + expect(Account.local).to match_array([account_1]) + end + end + + describe 'silenced' do + it 'returns an array of accounts who are silenced' do + account_1 = Fabricate(:account, silenced: true) + account_2 = Fabricate(:account, silenced: false) + expect(Account.silenced).to match_array([account_1]) + end + end + + describe 'suspended' do + it 'returns an array of accounts who are suspended' do + account_1 = Fabricate(:account, suspended: true) + account_2 = Fabricate(:account, suspended: false) + expect(Account.suspended).to match_array([account_1]) + end + end + end end diff --git a/spec/models/block_spec.rb b/spec/models/block_spec.rb index 6862de6fce..cabb41c3ea 100644 --- a/spec/models/block_spec.rb +++ b/spec/models/block_spec.rb @@ -1,5 +1,22 @@ require 'rails_helper' RSpec.describe Block, type: :model do + describe 'validations' do + it 'has a valid fabricator' do + block = Fabricate.build(:block) + expect(block).to be_valid + end + it 'is invalid without an account' do + block = Fabricate.build(:block, account: nil) + block.valid? + expect(block).to model_have_error_on_field(:account) + end + + it 'is invalid without a target_account' do + block = Fabricate.build(:block, target_account: nil) + block.valid? + expect(block).to model_have_error_on_field(:target_account) + end + end end diff --git a/spec/models/domain_block_spec.rb b/spec/models/domain_block_spec.rb index ad54031108..b19c8083ec 100644 --- a/spec/models/domain_block_spec.rb +++ b/spec/models/domain_block_spec.rb @@ -1,5 +1,23 @@ require 'rails_helper' RSpec.describe DomainBlock, type: :model do + describe 'validations' do + it 'has a valid fabricator' do + domain_block = Fabricate.build(:domain_block) + expect(domain_block).to be_valid + end + it 'is invalid without a domain' do + domain_block = Fabricate.build(:domain_block, domain: nil) + domain_block.valid? + expect(domain_block).to model_have_error_on_field(:domain) + end + + it 'is invalid if the domain already exists' do + domain_block_1 = Fabricate(:domain_block, domain: 'dalek.com') + domain_block_2 = Fabricate.build(:domain_block, domain: 'dalek.com') + domain_block_2.valid? + expect(domain_block_2).to model_have_error_on_field(:domain) + end + end end diff --git a/spec/models/follow_request_spec.rb b/spec/models/follow_request_spec.rb index f2ec642d80..cc6f8ee626 100644 --- a/spec/models/follow_request_spec.rb +++ b/spec/models/follow_request_spec.rb @@ -3,4 +3,23 @@ require 'rails_helper' RSpec.describe FollowRequest, type: :model do describe '#authorize!' describe '#reject!' + + describe 'validations' do + it 'has a valid fabricator' do + follow_request = Fabricate.build(:follow_request) + expect(follow_request).to be_valid + end + + it 'is invalid without an account' do + follow_request = Fabricate.build(:follow_request, account: nil) + follow_request.valid? + expect(follow_request).to model_have_error_on_field(:account) + end + + it 'is invalid without a target account' do + follow_request = Fabricate.build(:follow_request, target_account: nil) + follow_request.valid? + expect(follow_request).to model_have_error_on_field(:target_account) + end + end end diff --git a/spec/models/follow_spec.rb b/spec/models/follow_spec.rb index eb21f3e184..0fae253529 100644 --- a/spec/models/follow_spec.rb +++ b/spec/models/follow_spec.rb @@ -5,4 +5,23 @@ RSpec.describe Follow, type: :model do let(:bob) { Fabricate(:account, username: 'bob') } subject { Follow.new(account: alice, target_account: bob) } + + describe 'validations' do + it 'has a valid fabricator' do + follow = Fabricate.build(:follow) + expect(follow).to be_valid + end + + it 'is invalid without an account' do + follow = Fabricate.build(:follow, account: nil) + follow.valid? + expect(follow).to model_have_error_on_field(:account) + end + + it 'is invalid without a target_account' do + follow = Fabricate.build(:follow, target_account: nil) + follow.valid? + expect(follow).to model_have_error_on_field(:target_account) + end + end end diff --git a/spec/models/mention_spec.rb b/spec/models/mention_spec.rb index 5c91fda026..dbcf6a32c1 100644 --- a/spec/models/mention_spec.rb +++ b/spec/models/mention_spec.rb @@ -1,5 +1,22 @@ require 'rails_helper' RSpec.describe Mention, type: :model do + describe 'validations' do + it 'has a valid fabricator' do + mention = Fabricate.build(:mention) + expect(mention).to be_valid + end + it 'is invalid without an account' do + mention = Fabricate.build(:mention, account: nil) + mention.valid? + expect(mention).to model_have_error_on_field(:account) + end + + it 'is invalid without a status' do + mention = Fabricate.build(:mention, status: nil) + mention.valid? + expect(mention).to model_have_error_on_field(:status) + end + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 64de067497..1a32541850 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1,5 +1,49 @@ require 'rails_helper' RSpec.describe User, type: :model do + describe 'validations' do + it 'is invalid without an account' do + user = Fabricate.build(:user, account: nil) + user.valid? + expect(user).to model_have_error_on_field(:account) + end + it 'is invalid without a valid locale' do + user = Fabricate.build(:user, locale: 'toto') + user.valid? + expect(user).to model_have_error_on_field(:locale) + end + + it 'is invalid without a valid email' do + user = Fabricate.build(:user, email: 'john@') + user.valid? + expect(user).to model_have_error_on_field(:email) + end + end + + describe 'scopes' do + describe 'recent' do + it 'returns an array of recent users ordered by id' do + user_1 = Fabricate(:user) + user_2 = Fabricate(:user) + expect(User.recent).to match_array([user_2, user_1]) + end + end + + describe 'admins' do + it 'returns an array of users who are admin' do + user_1 = Fabricate(:user, admin: false) + user_2 = Fabricate(:user, admin: true) + expect(User.admins).to match_array([user_2]) + end + end + + describe 'confirmed' do + it 'returns an array of users who are confirmed' do + user_1 = Fabricate(:user, confirmed_at: nil) + user_2 = Fabricate(:user, confirmed_at: Time.now) + expect(User.confirmed).to match_array([user_2]) + end + end + end end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 977c7bdc08..faac96982d 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -8,6 +8,8 @@ require 'rspec/rails' require 'webmock/rspec' require 'paperclip/matchers' +Dir[Rails.root.join('spec/support/**/*.rb')].each { |f| require f } + ActiveRecord::Migration.maintain_test_schema! WebMock.disable_net_connect!(allow: 'localhost:7575') Sidekiq::Testing.inline! diff --git a/spec/support/matchers/model/model_have_error_on_field.rb b/spec/support/matchers/model/model_have_error_on_field.rb new file mode 100644 index 0000000000..5d5fe1c7b8 --- /dev/null +++ b/spec/support/matchers/model/model_have_error_on_field.rb @@ -0,0 +1,15 @@ +RSpec::Matchers.define :model_have_error_on_field do |expected| + match do |record| + if record.errors.empty? + record.valid? + end + + record.errors.has_key?(expected) + end + + failure_message do |record| + keys = record.errors.keys + + "expect record.errors(#{keys}) to include #{expected}" + end +end From 7762467b476af5c793d214ebc051b983921492b7 Mon Sep 17 00:00:00 2001 From: Samy KACIMI Date: Wed, 5 Apr 2017 00:31:31 +0200 Subject: [PATCH 2/6] rollback database.yml update --- config/database.yml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/config/database.yml b/config/database.yml index 3901134806..5ec342f939 100644 --- a/config/database.yml +++ b/config/database.yml @@ -3,10 +3,6 @@ default: &default pool: <%= ENV["DB_POOL"] || ENV['MAX_THREADS'] || 5 %> timeout: 5000 encoding: unicode - host: localhost - username: samy - password: tardis - port: 32769 development: <<: *default From 46c0e8b0e7980ecba0e68fe3b8c4d9121caa4b6f Mon Sep 17 00:00:00 2001 From: Samy KACIMI Date: Wed, 5 Apr 2017 00:37:23 +0200 Subject: [PATCH 3/6] update account_spec --- spec/models/account_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/account_spec.rb b/spec/models/account_spec.rb index fbc9a7d407..d7f59adb82 100644 --- a/spec/models/account_spec.rb +++ b/spec/models/account_spec.rb @@ -223,7 +223,7 @@ RSpec.describe Account, type: :model do expect(account).to model_have_error_on_field(:username) end - it 'is invalid is the username already exists' do + it 'is invalid if the username already exists' do account_1 = Fabricate(:account, username: 'the_doctor') account_2 = Fabricate.build(:account, username: 'the_doctor') account_2.valid? From 79ef756f645153b91643765573230814257d0cbf Mon Sep 17 00:00:00 2001 From: Samy KACIMI Date: Wed, 5 Apr 2017 00:47:17 +0200 Subject: [PATCH 4/6] fix rubocop issues --- Gemfile | 2 +- app/models/follow.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Gemfile b/Gemfile index 87ea77735c..0deed9ae01 100644 --- a/Gemfile +++ b/Gemfile @@ -69,8 +69,8 @@ end group :test do gem 'simplecov', require: false gem 'webmock' - gem 'rspec-sidekiq' gem 'faker' + gem 'rspec-sidekiq' end group :development do diff --git a/app/models/follow.rb b/app/models/follow.rb index fd7325f059..b6b9dca7cb 100644 --- a/app/models/follow.rb +++ b/app/models/follow.rb @@ -4,7 +4,7 @@ class Follow < ApplicationRecord include Paginable belongs_to :account, counter_cache: :following_count, required: true - + belongs_to :target_account, class_name: 'Account', counter_cache: :followers_count, From 5af0ecbcd9cfd757c4d5bd541d83ca11e44d14ef Mon Sep 17 00:00:00 2001 From: Samy KACIMI Date: Wed, 5 Apr 2017 00:52:55 +0200 Subject: [PATCH 5/6] alphebatically order test gem group as required by rubocop --- Gemfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Gemfile b/Gemfile index 0deed9ae01..4e7ff6621d 100644 --- a/Gemfile +++ b/Gemfile @@ -67,10 +67,10 @@ group :development, :test do end group :test do - gem 'simplecov', require: false - gem 'webmock' gem 'faker' gem 'rspec-sidekiq' + gem 'simplecov', require: false + gem 'webmock' end group :development do From 667ffafef8c8b7956cdd31b8f65d5e82778211d8 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Wed, 5 Apr 2017 03:31:26 +0200 Subject: [PATCH 6/6] Fix spec --- config/locales/en.yml | 2 +- spec/models/user_spec.rb | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/config/locales/en.yml b/config/locales/en.yml index 750af0b7a2..742219df99 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -5,8 +5,8 @@ en: about_this: About this instance apps: Apps business_email: 'Business e-mail:' - contact: Contact closed_registrations: Registrations are currently closed on this instance. + contact: Contact description_headline: What is %{domain}? domain_count_after: other instances domain_count_before: Connected to diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 5575ba107b..eb2a4aaeaf 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -45,8 +45,9 @@ RSpec.describe User, type: :model do expect(User.confirmed).to match_array([user_2]) end end + end - let(:account) { Fabricate(:account, username: 'alice') } + let(:account) { Fabricate(:account, username: 'alice') } let(:password) { 'abcd1234' } describe 'blacklist' do @@ -55,7 +56,7 @@ RSpec.describe User, type: :model do expect(user.valid?).to be_truthy end - + it 'should not allow a blacklisted user to be created' do user = User.new(email: 'foo@mvrht.com', account: account, password: password)