From f47527b128625b42a243269444e061521af64906 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miquel=20Sabat=C3=A9=20Sol=C3=A0?= Date: Thu, 14 Mar 2024 11:34:28 +0100 Subject: [PATCH] Added rubocop-minitest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miquel Sabaté Solà --- .rubocop.yml | 8 ++++ Gemfile | 1 + Gemfile.lock | 4 ++ config/environments/production.rb | 3 ++ test/application_system_test_case.rb | 1 + test/controllers/exports_controller_test.rb | 12 ++--- test/models/comment_test.rb | 3 +- test/models/search_test.rb | 51 +++++++++------------ test/models/thing_test.rb | 24 ++++------ test/models/user_test.rb | 16 +++---- test/system/comments_test.rb | 22 +++++---- test/system/exports_test.rb | 5 +- test/system/searches_test.rb | 19 ++++++-- test/system/shared_searches_test.rb | 6 ++- test/system/tags_test.rb | 3 ++ test/system/things_test.rb | 25 ++++++---- 16 files changed, 119 insertions(+), 84 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 555e7e8..89339fe 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -23,6 +23,7 @@ AllCops: require: - rubocop-rails - rubocop-performance + - rubocop-minitest # # Layout @@ -104,3 +105,10 @@ Style/AsciiComments: Naming/MethodParameterName: MinNameLength: 2 AllowedNames: [_, n] + +# +# Minitest +# + +Minitest/MultipleAssertions: + Max: 5 diff --git a/Gemfile b/Gemfile index e81d84d..28d2409 100644 --- a/Gemfile +++ b/Gemfile @@ -52,6 +52,7 @@ group :development do # Style gem 'rubocop', require: false + gem 'rubocop-minitest', require: false gem 'rubocop-performance', require: false gem 'rubocop-rails', require: false end diff --git a/Gemfile.lock b/Gemfile.lock index 088670d..e6ca36d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -237,6 +237,9 @@ GEM unicode-display_width (>= 2.4.0, < 3.0) rubocop-ast (1.30.0) parser (>= 3.2.1.0) + rubocop-minitest (0.34.5) + rubocop (>= 1.39, < 2.0) + rubocop-ast (>= 1.30.0, < 2.0) rubocop-performance (1.19.1) rubocop (>= 1.7.0, < 2.0) rubocop-ast (>= 0.4.0) @@ -315,6 +318,7 @@ DEPENDENCIES rails (~> 7.1.3) rails-i18n (~> 7.0.0) rubocop + rubocop-minitest rubocop-performance rubocop-rails selenium-webdriver diff --git a/config/environments/production.rb b/config/environments/production.rb index a9e4d6d..d148180 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -90,6 +90,9 @@ # Don't log any deprecations. config.active_support.report_deprecations = false + # In our case having SQLite in production is actually just fine! + config.active_record.sqlite3_production_warning = false + # Do not dump schema after migrations. config.active_record.dump_schema_after_migration = false diff --git a/test/application_system_test_case.rb b/test/application_system_test_case.rb index eaa28b0..5fd0889 100644 --- a/test/application_system_test_case.rb +++ b/test/application_system_test_case.rb @@ -19,6 +19,7 @@ def sign_in! # Sign out the current user if it's already signed in, otherwise do nothing. def sign_out_maybe! click_on I18n.t('sessions.sign-out') + assert_text I18n.t('sessions.title') rescue Capybara::ElementNotFound # We were not logged in, do nothing. diff --git a/test/controllers/exports_controller_test.rb b/test/controllers/exports_controller_test.rb index 42ed1a2..9e69df8 100644 --- a/test/controllers/exports_controller_test.rb +++ b/test/controllers/exports_controller_test.rb @@ -8,17 +8,17 @@ class ExportsControllerTest < ActionDispatch::IntegrationTest get search_exports_url(0, format: 'csv') body = @response.body.split("\n") - assert body.size == 2 - assert body.first.split(',')[0] == things(:thing2).target - assert body.last.split(',')[0] == things(:thing1).target + targets = body.map { |b| b.split(',').first } - assert @response.media_type == 'text/csv' + assert_equal targets, [things(:thing2).target, things(:thing1).target] + + assert_equal 'text/csv', @response.media_type end test 'uoc: works' do get search_exports_url(0, format: 'uoc') - assert @response.body == file_fixture('all.uoc.tex').read - assert @response.media_type == 'application/x-tex' + assert_equal @response.body, file_fixture('all.uoc.tex').read + assert_equal 'application/x-tex', @response.media_type end end diff --git a/test/models/comment_test.rb b/test/models/comment_test.rb index 087963a..deb1a1d 100644 --- a/test/models/comment_test.rb +++ b/test/models/comment_test.rb @@ -13,6 +13,7 @@ class CommentTest < ActiveSupport::TestCase test 'has many tags through tag_references' do comment = comments(:comment1) - assert comment.tags.size == 1 + + assert_equal 1, comment.tags.size end end diff --git a/test/models/search_test.rb b/test/models/search_test.rb index 9614bc0..355beab 100644 --- a/test/models/search_test.rb +++ b/test/models/search_test.rb @@ -3,7 +3,7 @@ require 'test_helper' class SearchTest < ActiveSupport::TestCase - test 'name and body have to be present and unique' do + test 'name and body have to be present' do search = Search.new(user_id: users(:user).id) assert_raise(ActiveRecord::RecordInvalid) { search.save! } @@ -13,6 +13,10 @@ class SearchTest < ActiveSupport::TestCase search.name = nil search.body = 'body' assert_raise(ActiveRecord::RecordInvalid) { search.save! } + end + + test 'name and body have to be unique' do + search = Search.new(user_id: users(:user).id) search.name = searches(:search1).name assert_raise(ActiveRecord::RecordInvalid) { search.save! } @@ -20,9 +24,6 @@ class SearchTest < ActiveSupport::TestCase search.name = 'another' search.body = searches(:search1).body assert_raise(ActiveRecord::RecordInvalid) { search.save! } - - search.body = 'another body' - assert_difference('Search.count') { search.save! } end ## @@ -31,60 +32,52 @@ class SearchTest < ActiveSupport::TestCase test 'returns all things when an empty body is given' do res = Search.new.results - assert res[:things].size == 2 - assert res[:things][0][:target] == things(:thing2).target - assert res[:things][1][:target] == things(:thing1).target + assert_equal 2, res[:things].size + assert_equal res[:things][0][:target], things(:thing2).target + assert_equal res[:things][1][:target], things(:thing1).target end test 'returns everything matching a specific tag' do res = Search.new(body: "tag:'#{tags(:tag1).name}'").results - assert res[:things].size == 2 - assert res[:things][0][:target] == things(:thing2).target - assert res[:things][1][:target] == things(:thing1).target - assert res[:comments].size == 1 - assert res[:comments][0][:id] == comments(:comment1).id + assert_equal res[:things].map(&:target), [things(:thing2).target, things(:thing1).target] + assert_equal res[:comments].map(&:id), [comments(:comment1).id] end test 'returns everything matching two tags' do res = Search.new(body: "tag:'#{tags(:tag1).name}' tag:'#{tags(:tag2).name}'").results - assert res[:things].size == 1 - assert res[:things][0][:target] == things(:thing1).target - assert res[:comments].empty? + assert_equal 1, res[:things].size + assert_equal res[:things][0][:target], things(:thing1).target + assert_empty res[:comments] end test 'returns everything matching a given text' do res = Search.new(body: 'Miquel').results - assert res[:things].size == 2 - assert res[:things][0][:target] == things(:thing2).target - assert res[:things][1][:target] == things(:thing1).target - assert res[:comments].size == 1 - assert res[:comments][0][:id] == comments(:comment1).id + assert_equal res[:things].map(&:target), [things(:thing2).target, things(:thing1).target] + assert_equal res[:comments].map(&:id), [comments(:comment1).id] end test 'returns everything matching two tags and a given text' do res = Search.new(body: "Miquel tag:'#{tags(:tag1).name}' tag:'#{tags(:tag2).name}'").results - assert res[:things].size == 1 - assert res[:things][0][:target] == things(:thing1).target - assert res[:comments].empty? + assert_equal 1, res[:things].size + assert_equal res[:things][0][:target], things(:thing1).target + assert_empty res[:comments] end test 'returns everything matching a given compound text' do res = Search.new(body: 'some other').results - assert res[:things].size == 2 - assert res[:things][0][:target] == things(:thing2).target - assert res[:things][1][:target] == things(:thing1).target - assert res[:comments].empty? + assert_equal res[:things].map(&:target), [things(:thing2).target, things(:thing1).target] + assert_empty res[:comments] end test 'returns an empty result with unknown fields' do res = Search.new(body: 'whatever:"unknown"').results - assert res[:things].empty? - assert res[:comments].empty? + assert_empty res[:things] + assert_empty res[:comments] end end diff --git a/test/models/thing_test.rb b/test/models/thing_test.rb index 0b3961e..766a5e2 100644 --- a/test/models/thing_test.rb +++ b/test/models/thing_test.rb @@ -3,36 +3,28 @@ require 'test_helper' class ThingTest < ActiveSupport::TestCase - test 'validates presence' do - assert_raise(ActiveRecord::RecordInvalid) { Thing.new.save! } - - # Missing 'title' + test 'presence: title' do assert_raise(ActiveRecord::RecordInvalid) do Thing.new(target: 'target', authors: 'Author', user_id: users(:user).id, rate: 5, status: Thing.statuses[:read], kind: Thing.kinds[:novel]).save! end + end - # Missing 'target' + test 'presence: target' do assert_raise(ActiveRecord::RecordInvalid) do Thing.new(title: 'title', authors: 'Author', user_id: users(:user).id, rate: 5, status: Thing.statuses[:read], kind: Thing.kinds[:novel]).save! end + end - # Missing 'authors' + test 'presence: authors' do assert_raise(ActiveRecord::RecordInvalid) do Thing.new(title: 'title', target: 'target', user_id: users(:user).id, rate: 5, status: Thing.statuses[:read], kind: Thing.kinds[:novel]).save! end - - # Valid! - assert_difference('Thing.count') do - Thing.new(title: 'title', target: 'target', authors: 'Author', - user_id: users(:user).id, rate: 5, - status: Thing.statuses[:read], kind: Thing.kinds[:novel]).save! - end end test "'rate' has to be between 0 and 10" do @@ -85,11 +77,13 @@ class ThingTest < ActiveSupport::TestCase test 'has many comments' do thing = things(:thing1) - assert thing.comments.size == 1 + + assert_equal 1, thing.comments.size end test 'has many tags through tag_references' do thing = things(:thing1) - assert thing.tags.size == 2 + + assert_equal 2, thing.tags.size end end diff --git a/test/models/user_test.rb b/test/models/user_test.rb index 6ff5e19..e22afa7 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -3,21 +3,19 @@ require 'test_helper' class UserTest < ActiveSupport::TestCase - test 'has to provide username and password' do + test 'presence: username' do user = User.new + user.password = '1234' + user.password_confirmation = '12345' + assert_raise(ActiveRecord::RecordInvalid) { user.save! } + end + test 'presence: password' do + user = User.new user.username = 'whatever' - assert_raise(ActiveRecord::RecordInvalid) { user.save! } - user.password = '1234' - user.password_confirmation = '12345' assert_raise(ActiveRecord::RecordInvalid) { user.save! } - - user.password_confirmation = '1234' - assert_difference('User.count') do - user.save! - end end test 'username has to be unique' do diff --git a/test/system/comments_test.rb b/test/system/comments_test.rb index b2490b4..33a1398 100644 --- a/test/system/comments_test.rb +++ b/test/system/comments_test.rb @@ -12,9 +12,11 @@ class CommentsTest < ApplicationSystemTestCase visit thing_url(things(:thing1)) click_on I18n.t('comments.new') + assert_text I18n.t('tags.new-action'), count: 1 click_on I18n.t('general.cancel') + assert_text I18n.t('tags.new-action'), count: 0 end @@ -28,10 +30,9 @@ class CommentsTest < ApplicationSystemTestCase click_on I18n.t('helpers.submit.create') - assert_text "#{I18n.t('comments.title')} #2" assert_text 'a new comment' assert_text "#{I18n.t('tags.title')}: #{tags(:tag1).name}, #{tags(:tag2).name}" - assert page.current_path == thing_path(things(:thing1)) + assert_equal page.current_path, thing_path(things(:thing1)) end test 'things#show: you can update a comment' do @@ -41,16 +42,18 @@ class CommentsTest < ApplicationSystemTestCase find(:css, '#comment_content').set('updated comment') find("#comment_#{comments(:comment1).id} input[type=submit]").click + assert_text 'updated comment' - assert page.current_path == thing_path(things(:thing1)) + assert_equal page.current_path, thing_path(things(:thing1)) end test 'things#show: you can delete a comment' do visit thing_url(things(:thing1)) all('.delete-comment').last.click + assert_text I18n.t('comments.none') - assert page.current_path == thing_path(things(:thing1)) + assert_equal page.current_path, thing_path(things(:thing1)) end ## @@ -60,9 +63,11 @@ class CommentsTest < ApplicationSystemTestCase visit edit_thing_url(things(:thing1)) click_on I18n.t('comments.new') + assert_text I18n.t('tags.new-action'), count: 2 click_on I18n.t('general.cancel') + assert_text I18n.t('tags.new-action'), count: 1 end @@ -76,10 +81,9 @@ class CommentsTest < ApplicationSystemTestCase click_on I18n.t('helpers.submit.create') - assert_text "#{I18n.t('comments.title')} #2" assert_text 'a new comment' assert_text "#{I18n.t('tags.title')}: #{tags(:tag1).name}, #{tags(:tag2).name}" - assert page.current_path == edit_thing_path(things(:thing1)) + assert_equal page.current_path, edit_thing_path(things(:thing1)) end test 'things#edit: you can update a comment' do @@ -89,15 +93,17 @@ class CommentsTest < ApplicationSystemTestCase find(:css, '#comment_content').set('updated comment') find("#comment_#{comments(:comment1).id} input[type=submit]").click + assert_text 'updated comment' - assert page.current_path == edit_thing_path(things(:thing1)) + assert_equal page.current_path, edit_thing_path(things(:thing1)) end test 'things#edit: you can delete a comment' do visit edit_thing_url(things(:thing1)) all('.delete-comment').last.click + assert_text I18n.t('comments.none') - assert page.current_path == edit_thing_path(things(:thing1)) + assert_equal page.current_path, edit_thing_path(things(:thing1)) end end diff --git a/test/system/exports_test.rb b/test/system/exports_test.rb index b324321..fea3dad 100644 --- a/test/system/exports_test.rb +++ b/test/system/exports_test.rb @@ -9,11 +9,13 @@ class ExportsTest < ApplicationSystemTestCase test 'can access exports from the hidden menu' do find('#toggle-hidden-global-menu').click + assert_text I18n.t('searches.export.action') click_on I18n.t('searches.export.action') + assert_text I18n.t('searches.export.note-msg') - assert page.current_path == new_search_exports_path(0) + assert_equal page.current_path, new_search_exports_path(0) end test 'can export using multiple formats' do @@ -22,6 +24,7 @@ class ExportsTest < ApplicationSystemTestCase assert find('#export-format-button')[:href].ends_with?('/searches/0/exports.csv') find('#export-select-format').select('UOC') + assert find('#export-format-button')[:href].ends_with?('/searches/0/exports.uoc') end end diff --git a/test/system/searches_test.rb b/test/system/searches_test.rb index a400e71..309d51c 100644 --- a/test/system/searches_test.rb +++ b/test/system/searches_test.rb @@ -21,11 +21,13 @@ class SearchesTest < ApplicationSystemTestCase test 'the hidden menu should allow us to go into the searches#new url' do find('#toggle-hidden-global-menu').click + assert_text I18n.t('searches.title') click_on I18n.t('searches.object') + assert find('#search_body') - assert page.current_path == new_search_path + assert_equal page.current_path, new_search_path end test 'can perform a new search' do @@ -56,22 +58,25 @@ class SearchesTest < ApplicationSystemTestCase # Hit the save button so the hidden form appears. assert_text I18n.t('searches.save') find('#save-search').click + assert_text I18n.t('searches.public') fill_in 'Name', with: 'Nova cerca' click_on I18n.t('helpers.submit.create') assert_text 'Nova cerca' - assert page.current_path == search_path(Search.find_by(name: 'Nova cerca')) + assert_equal page.current_path, search_path(Search.find_by(name: 'Nova cerca')) end test 'can edit a search that is not the Home' do # Home cannot be edited. find('#toggle-hidden-global-menu').click + assert_selector 'a', text: I18n.t('general.edit'), count: 0 # Select search1 click_on searches(:search1).name + assert_selector 'a', text: things(:thing1).title, count: 2 find('#toggle-hidden-global-menu').click @@ -81,7 +86,7 @@ class SearchesTest < ApplicationSystemTestCase # If you click on it, you will be on the edit page for it. assert find('#search_body') - assert page.current_path == edit_search_path(searches(:search1)) + assert_equal page.current_path, edit_search_path(searches(:search1)) end test 'edit an existing search works' do @@ -92,6 +97,7 @@ class SearchesTest < ApplicationSystemTestCase # Hit the save button so the hidden form appears. assert_text I18n.t('searches.save') find('#save-search').click + assert_text I18n.t('searches.public') # Update the name. @@ -101,23 +107,26 @@ class SearchesTest < ApplicationSystemTestCase # We have been redirected to search#show, and the name has been refreshed. assert_selector 'a', text: searches(:search1).name, count: 0 assert_text 'Nova cerca' - assert page.current_path == search_path(searches(:search1)) + assert_equal page.current_path, search_path(searches(:search1)) end test 'can delete a search that is not the Home' do # Home cannot be deleted. find('#toggle-hidden-global-menu').click + assert_selector 'a', text: I18n.t('general.delete'), count: 0 # Select search1 click_on searches(:search1).name + assert_selector 'a', text: things(:thing1).title, count: 2 find('#toggle-hidden-global-menu').click # It can be deleted! assert_text I18n.t('general.delete') accept_alert { click_on I18n.t('general.delete') } + assert_selector 'a', text: searches(:search1).name, count: 0 - assert Search.none? + assert_predicate Search, :none? end end diff --git a/test/system/shared_searches_test.rb b/test/system/shared_searches_test.rb index 0cfc4bd..6828dbe 100644 --- a/test/system/shared_searches_test.rb +++ b/test/system/shared_searches_test.rb @@ -7,9 +7,11 @@ class SharedSearchesTest < ApplicationSystemTestCase sign_in! find('#toggle-hidden-global-menu').click + assert_text I18n.t('searches.shared.title').downcase click_on I18n.t('searches.shared.title').downcase + assert_text I18n.t('searches.shared.none') end @@ -17,8 +19,9 @@ class SharedSearchesTest < ApplicationSystemTestCase sign_out_maybe! visit shared_searches_path + assert_text I18n.t('sessions.title') - assert page.current_path == new_sessions_path + assert_equal page.current_path, new_sessions_path end test 'shared_searches#show gives us the expected results' do @@ -27,6 +30,7 @@ class SharedSearchesTest < ApplicationSystemTestCase searches(:search1).update!(shared: true) visit search_shared_path(searches(:search1)) + assert_text things(:thing1).title assert_text things(:thing2).title assert_text I18n.t('comments.title') diff --git a/test/system/tags_test.rb b/test/system/tags_test.rb index 1ee83a9..8789087 100644 --- a/test/system/tags_test.rb +++ b/test/system/tags_test.rb @@ -17,6 +17,7 @@ class SharedSearchesTest < ApplicationSystemTestCase fill_in I18n.t('activerecord.attributes.tag.name'), with: 'whatever' assert_difference 'Tag.count' do click_on I18n.t('helpers.submit.create') + assert_text 'whatever' end end @@ -26,6 +27,7 @@ class SharedSearchesTest < ApplicationSystemTestCase fill_in I18n.t('activerecord.attributes.tag.name'), with: tags(:tag1).name click_on I18n.t('helpers.submit.create') + assert_text "#{I18n.t('activerecord.attributes.tag.name')} #{I18n.t('errors.messages.taken')}" end @@ -34,6 +36,7 @@ class SharedSearchesTest < ApplicationSystemTestCase assert_difference 'Tag.count', -1 do click_link(I18n.t('general.delete'), match: :first) + assert_text I18n.t('tags.title') end end diff --git a/test/system/things_test.rb b/test/system/things_test.rb index 9dd482d..792c07f 100644 --- a/test/system/things_test.rb +++ b/test/system/things_test.rb @@ -7,12 +7,14 @@ class ThingsTest < ApplicationSystemTestCase test 'the hidden menu has a things#new link' do find('#toggle-hidden-global-menu').click + assert_text I18n.t('things.object').downcase click_on I18n.t('things.object').downcase + assert_text I18n.t('activerecord.attributes.thing.title') - assert page.current_path == new_thing_path + assert_equal page.current_path, new_thing_path end test 'you can click on a thing listed on the search to go to the its #show page' do @@ -20,7 +22,7 @@ class ThingsTest < ApplicationSystemTestCase assert_text I18n.t('activerecord.attributes.thing.title') - assert page.current_path == edit_thing_path(things(:thing1)) + assert_equal page.current_path, edit_thing_path(things(:thing1)) end test 'you can create a new thing' do @@ -38,13 +40,17 @@ class ThingsTest < ApplicationSystemTestCase # Ensure that tag references are properly created. tags = Thing.find_by(title: 'new title').tags - assert tags.size == 1 - assert tags.first.name == tags(:tag1).name - # You can go back to create another thing with a convenient link. + assert_equal tags.map(&:name), [tags(:tag1).name] + end + + test 'you have a link to go back at creating a thing' do + visit thing_url(things(:thing1)) + click_on I18n.t('things.create-another') + assert_text I18n.t('activerecord.attributes.thing.title') - assert page.current_path == new_thing_path + assert_equal page.current_path, new_thing_path end test 'you get feedback from wrong values for a new thing' do @@ -67,7 +73,7 @@ class ThingsTest < ApplicationSystemTestCase # Originally :thing1 has references to :tag1 and :tag2. This test will # remove the reference to :tag1. - assert things(:thing1).tag_references.size == 2 + assert_equal 2, things(:thing1).tag_references.size fill_in I18n.t('activerecord.attributes.thing.title'), with: 'another thing entirely' find("#thing_tag_ids_#{tags(:tag1).id}").click @@ -77,8 +83,8 @@ class ThingsTest < ApplicationSystemTestCase # Tag references updated: only one reference (:tag2). tags = things(:thing1).reload.tags - assert tags.size == 1 - assert tags.first.id = tags(:tag2).id + + assert_equal tags.map(&:id), [tags(:tag2).id] end test 'you get feedback from wrong values for a thing' do @@ -96,6 +102,7 @@ class ThingsTest < ApplicationSystemTestCase assert_difference 'Thing.count', -1 do accept_alert { click_on I18n.t('general.delete').capitalize, match: :first } + assert_text I18n.t('things.destroy-success') end end