From 0e6c71f7ca29962f2e59eb866136ecedc0398602 Mon Sep 17 00:00:00 2001 From: Nicolas Rodriguez Date: Tue, 3 Sep 2024 18:04:46 +0200 Subject: [PATCH] Improve Rubocop config, fix offenses --- .rubocop.yml | 23 ++++- spec/config_capybara.rb | 2 + spec/config_rspec.rb | 8 +- spec/remotipart/features/comments_spec.rb | 103 +++++++++++---------- spec/remotipart/features/prepended_spec.rb | 8 +- spec/support/connection_helper.rb | 6 +- spec/support/integration_helper.rb | 6 +- 7 files changed, 95 insertions(+), 61 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 61c3c8e..be3fc08 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -13,7 +13,7 @@ AllCops: - bin/* - lib/generators/**/*.rb - gemfiles/* - - spec/**/* + - spec/dummy/**/* Gemspec/RequireMFA: Enabled: false @@ -36,6 +36,7 @@ Layout/LineLength: Max: 130 Exclude: - remotipart.gemspec + - spec/remotipart/features/comments_spec.rb Layout/EmptyLines: Enabled: false @@ -58,3 +59,23 @@ Layout/EmptyLinesAroundModuleBody: Naming/BlockForwarding: Enabled: false + +######### +# RSPEC # +######### + +RSpec/MetadataStyle: + EnforcedStyle: hash + +RSpec/ExampleLength: + Exclude: + - spec/remotipart/features/comments_spec.rb + +RSpec/MultipleExpectations: + Max: 9 + +RSpec/NotToNot: + EnforcedStyle: to_not + +Capybara/ClickLinkOrButtonStyle: + EnforcedStyle: strict diff --git a/spec/config_capybara.rb b/spec/config_capybara.rb index 0fde836..c58710b 100644 --- a/spec/config_capybara.rb +++ b/spec/config_capybara.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + def register_driver(driver_name, args = []) opts = { js_errors: true, headless: true, window_size: [1920, 1200], browser_options: {} } args.each do |arg| diff --git a/spec/config_rspec.rb b/spec/config_rspec.rb index 0cf4b2c..9cfb1c4 100644 --- a/spec/config_rspec.rb +++ b/spec/config_rspec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # Configure RSpec RSpec.configure do |config| config.include Rails.application.routes.url_helpers @@ -31,16 +33,16 @@ DatabaseCleaner.strategy = :truncation end - config.before(:each) do + config.before do DatabaseCleaner.start end - config.after(:each) do + config.after do DatabaseCleaner.clean end if ENV.key?('GITHUB_ACTIONS') - config.around(:each) do |ex| + config.around do |ex| ex.run_with_retry retry: 3 end end diff --git a/spec/remotipart/features/comments_spec.rb b/spec/remotipart/features/comments_spec.rb index 4302b05..d070632 100644 --- a/spec/remotipart/features/comments_spec.rb +++ b/spec/remotipart/features/comments_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' RSpec.describe 'comments', type: :feature do @@ -31,7 +33,7 @@ expect(page).to have_link('Cancel') end - it "cancels creating a comment", js: true do + it 'cancels creating a comment', js: true do visit root_path click_link 'New Comment' @@ -44,7 +46,7 @@ expect(page).to have_link('New Comment') end - it "deletes a comment", js: true do + it 'deletes a comment', js: true do Comment.create(subject: 'The Great Yogurt', body: 'The Schwarz is strong with this one.') visit root_path @@ -58,7 +60,7 @@ end end - it "uploads a file", js: true do + it 'uploads a file', js: true do visit root_path click_link 'New Comment with Attachment' @@ -83,19 +85,20 @@ end within '#comments' do - expect(page).to have_selector("td", text: comment_subject) - expect(page).to have_selector("td", text: comment_body) - expect(page).to have_selector("a", text: File.basename(file_path)) - expect(page).to have_selector("a", text: File.basename(other_file_path)) + expect(page).to have_css('td', text: comment_subject) + expect(page).to have_css('td', text: comment_body) + expect(page).to have_css('a', text: File.basename(file_path)) + expect(page).to have_css('a', text: File.basename(other_file_path)) end end - it "Disables submit button while submitting", js: true do + it 'Disables submit button while submitting', js: true do visit root_path click_link 'New Comment' + # Needed to make test wait for above to finish - form = find('form') + find('form') button = find_button('Create Comment') page.execute_script(%q{$('form').append('');}) @@ -105,15 +108,15 @@ click_button 'Create Comment' expect(button[:disabled]).to be true - expect(button.value).to eq "Submitting..." + expect(button.value).to eq 'Submitting...' sleep 1.5 expect(button[:disabled]).to be false - expect(button.value).to eq "Create Comment" + expect(button.value).to eq 'Create Comment' end - it "triggers ajax:remotipartSubmit event hook", js: true do + it 'triggers ajax:remotipartSubmit event hook', js: true do visit root_path page.execute_script("$(document).delegate('form', 'ajax:remotipartSubmit', function() { $('#comments').after('remotipart!'); });") @@ -127,7 +130,7 @@ expect(page).to have_content('remotipart!') end - it "allows remotipart submission to be cancelable via event hook", js: true do + it 'allows remotipart submission to be cancelable via event hook', js: true do visit root_path page.execute_script("$(document).delegate('form', 'ajax:remotipartSubmit', function() { $('#comments').after('remotipart!'); return false; });") @@ -148,14 +151,14 @@ end end - it "allows custom data-type on form", js: true do + it 'allows custom data-type on form', js: true do visit root_path page.execute_script("$(document).delegate('form', 'ajax:success', function(evt, data, status, xhr) { $('#comments').after(xhr.responseText); });") click_link 'New Comment with Attachment' # Needed to make test wait for above to finish - form = find('form') + find('form') page.execute_script("$('form').attr('data-type', 'html');") file_path = File.join(fixture_path, 'qr.jpg') @@ -167,14 +170,14 @@ expect(page).to have_content('HTML response') end - it "allows users to use ajax response data safely", js: true do + it 'allows users to use ajax response data safely', js: true do visit root_path page.execute_script("$(document).delegate('form', 'ajax:success', function(evt, data, status, xhr) { $('#comments').after(data); });") click_link 'New Comment with Attachment' # Needed to make test wait for above to finish - form = find('form') + find('form') page.execute_script("$('form').attr('data-type', 'html');") file_path = File.join(fixture_path, 'qr.jpg') @@ -186,14 +189,14 @@ expect(page).to have_content('HTML response') end - it "escapes html response content properly", js: true do + it 'escapes html response content properly', js: true do visit root_path page.execute_script("$(document).delegate('form', 'ajax:success', function(evt, data, status, xhr) { $('#comments').after(xhr.responseText); });") click_link 'New Comment with Attachment' # Needed to make test wait for above to finish - form = find('form') + find('form') page.execute_script("$('form').attr('data-type', 'html');") page.execute_script("$('form').append('');") @@ -206,12 +209,12 @@ expect(find('input[name="quote"]').value).to eq '"' end - it "returns the correct response status", js: true do + it 'returns the correct response status', js: true do visit root_path click_link 'New Comment with Attachment' # Needed to make test wait for above to finish - input = find('#comment_subject') + find_by_id('comment_subject') page.execute_script("$('#comment_subject').removeAttr('required');") file_path = File.join(fixture_path, 'qr.jpg') @@ -219,16 +222,16 @@ attach_file 'comment_attachment', file_path click_button 'Create Comment' - #within '#error_explanation' do - # expect(page).to have_content "Subject can't be blank" - #end - expect(page).to have_content "Error status code: 422" + # within '#error_explanation' do + # expect(page).to have_content "Subject can't be blank" + # end + expect(page).to have_content 'Error status code: 422' - message = Rails.version >= "7.1" ? "Content" : "Entity" + message = Rails.version >= '7.1' ? 'Content' : 'Entity' expect(page).to have_content "Error status message: Unprocessable #{message}" end - it "passes the method as _method parameter (rails convention)", js: true do + it 'passes the method as _method parameter (rails convention)', js: true do visit root_path click_link 'New Comment with Attachment' @@ -244,7 +247,7 @@ expect(page).to have_content 'PUT request!' end - it "does not submit via remotipart unless file is present", js: true do + it 'does not submit via remotipart unless file is present', js: true do visit root_path page.execute_script("$(document).delegate('form', 'ajax:remotipartSubmit', function() { $('#comments').after('remotipart!'); });") @@ -257,12 +260,12 @@ expect(page).to have_no_content('remotipart!') end - it "fires all the ajax callbacks on the form", js: true do + it 'fires all the ajax callbacks on the form', js: true do visit root_path click_link 'New Comment with Attachment' # Needed to make test wait for above to finish - form = find('form') + find('form') page.execute_script("$('form').bind('ajax:beforeSend', function() { $('#comments').after('thebefore'); });") page.execute_script("$(document).delegate('form', 'ajax:success', function() { $('#comments').after('success'); });") @@ -279,12 +282,12 @@ expect(page).to have_content('complete') end - it "fires the ajax callbacks for json data-type with remotipart", js: true do + it 'fires the ajax callbacks for json data-type with remotipart', js: true do visit root_path click_link 'New Comment with Attachment' # Needed to make test wait for above to finish - form = find('form') + find('form') page.execute_script("$('form').data('type', 'json');") @@ -303,12 +306,12 @@ expect(page).to have_content('complete') end - it "only fires the beforeSend hook once", js: true do + it 'only fires the beforeSend hook once', js: true do visit root_path click_link 'New Comment with Attachment' # Needed to make test wait for above to finish - form = find('form') + find('form') page.execute_script("$('form').bind('ajax:beforeSend', function() { $('#comments').after('
ajax!
'); });") @@ -318,10 +321,10 @@ attach_file 'comment_attachment', file_path click_button 'Create Comment' - expect(page).to have_css("div.ajax", :count => 1) + expect(page).to have_css('div.ajax', count: 1) end - it "cleans up after itself when uploading files", js: true do + it 'cleans up after itself when uploading files', js: true do visit root_path page.execute_script("$(document).delegate('form', 'ajax:remotipartSubmit', function(evt, xhr, data) { if ($(this).data('remotipartSubmitted')) { $('#comments').after('remotipart before!'); } });") @@ -340,7 +343,7 @@ expect(page).to have_content('no remotipart after!') end - it "submits via remotipart when a file upload is present", js: true do + it 'submits via remotipart when a file upload is present', js: true do visit root_path page.execute_script("$(document).delegate('form', 'ajax:remotipartSubmit', function(evt, xhr, data) { $('#comments').after('
remotipart!
'); });") @@ -353,10 +356,10 @@ attach_file 'comment_attachment', file_path click_button 'Create Comment' - expect(page).to have_css("div.remotipart") + expect(page).to have_css('div.remotipart') end - it "does not submit via remotipart when a file upload is not present", js: true do + it 'does not submit via remotipart when a file upload is not present', js: true do visit root_path page.execute_script("$(document).delegate('form', 'ajax:remotipartSubmit', function(evt, xhr, data) { $('#comments').after('
remotipart!
'); });") @@ -367,11 +370,11 @@ fill_in 'comment_body', with: 'there' click_button 'Create Comment' - expect(page).not_to have_css("div.remotipart") + expect(page).to have_no_css('div.remotipart') end - it "Disables submit button while submitting with remotipart", js: true do - skip "actually it works" + it 'Disables submit button while submitting with remotipart', js: true do + skip 'actually it works' visit root_path @@ -392,18 +395,18 @@ attach_file 'comment_attachment', file_path click_button 'Create Comment' - expect(page.evaluate_script("window.commitButtonDisabled")).to be true - expect(page.evaluate_script("window.commitButtonValue")).to eq "Submitting..." + expect(page.evaluate_script('window.commitButtonDisabled')).to be true + expect(page.evaluate_script('window.commitButtonValue')).to eq 'Submitting...' expect(button[:disabled]).to be false - expect(button.value).to eq "Create Comment" + expect(button.value).to eq 'Create Comment' end - it "submits the clicked button with the form like non-file remote form", js: true do + it 'submits the clicked button with the form like non-file remote form', js: true do visit root_path click_link 'New Comment with Attachment' - form = find('form') + find('form') page.execute_script("$('form').bind('ajax:remotipartSubmit', function(e, xhr, settings) { $('#comments').after('
' + $.param(settings.data) + '
'); });") file_path = File.join(fixture_path, 'qr.jpg') @@ -416,8 +419,8 @@ end it "doesn't allow XSS via script injection for text responses", js: true do - visit "/say?message=%3C/textarea%3E%3Csvg/onload=alert(domain)%3E&remotipart_submitted=x" - expect(page).to have_selector("textarea") - expect(find("textarea").value).to eq('') + visit '/say?message=%3C/textarea%3E%3Csvg/onload=alert(domain)%3E&remotipart_submitted=x' + expect(page).to have_css('textarea') + expect(find('textarea').value).to eq('') end end diff --git a/spec/remotipart/features/prepended_spec.rb b/spec/remotipart/features/prepended_spec.rb index f381826..b90508d 100644 --- a/spec/remotipart/features/prepended_spec.rb +++ b/spec/remotipart/features/prepended_spec.rb @@ -1,9 +1,11 @@ +# frozen_string_literal: true + require 'spec_helper' RSpec.describe 'prepended', type: :feature do - context "when another library overrides #render using prepend" do - it "does not break" do - expect { visit prepended_path }.not_to raise_error + context 'when another library overrides #render using prepend' do + it 'does not break' do + expect { visit prepended_path }.to_not raise_error end end end diff --git a/spec/support/connection_helper.rb b/spec/support/connection_helper.rb index 032520c..d2bfe2d 100644 --- a/spec/support/connection_helper.rb +++ b/spec/support/connection_helper.rb @@ -1,6 +1,8 @@ -class ActiveRecord::Base +# frozen_string_literal: true + +class ActiveRecord::Base # rubocop:disable Style/ClassAndModuleChildren mattr_accessor :shared_connection - @@shared_connection = nil + @@shared_connection = nil # rubocop:disable Style/ClassVars def self.connection @@shared_connection || retrieve_connection diff --git a/spec/support/integration_helper.rb b/spec/support/integration_helper.rb index d3e0119..381ccac 100644 --- a/spec/support/integration_helper.rb +++ b/spec/support/integration_helper.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module IntegrationHelper # If you do something that triggers a confirm, do it inside an accept_js_confirm or reject_js_confirm block def accept_js_confirm @@ -19,10 +21,10 @@ def reject_js_confirm # test this, I just don't know it. def page_should_not_redirect path = current_path - text = "bleep bloop" + text = 'bleep bloop' page.execute_script "var txt = document.createTextNode('#{text}');document.body.appendChild(txt);" yield - expect(current_path).to eq path + expect(page).to have_current_path(path, ignore_query: true) expect(page).to have_content(text) end end