Merge pull request #10757 from opf/fix/name_spec_files_correctly

Fix spec path to end with *_spec.rb

modules/auth_plugins/spec/requests/auth_plugins_spec.rb

@@ -30,10 +30,9 @@ require 'spec_helper'
 require 'open_project/auth_plugins'
 
 describe OpenProject::Plugins::AuthPlugin do
-  class MockEngine
-    extend OpenProject::Plugins::AuthPlugin
+  let(:dummy_engine_klass) do
+    Class.new { extend OpenProject::Plugins::AuthPlugin }
   end
-
   let(:strategies) { {} }
   let(:providers_a) do
     lambda { [{ name: 'a1' }, { name: 'a2' }] }
@@ -48,6 +47,7 @@ describe OpenProject::Plugins::AuthPlugin do
   let(:middlewares) { [] }
 
   before do
+    with_enterprise_token :board_view
     app = Object.new
     omniauth_builder = Object.new
 
@@ -55,13 +55,13 @@ describe OpenProject::Plugins::AuthPlugin do
       middlewares << strategy
     }
 
-    allow(app).to receive_message_chain(:config, :middleware, :use) { |_mw, &block|
+    allow(app).to receive_message_chain(:config, :middleware, :use) { |_mw, &block| # rubocop:disable RSpec/MessageChain
       omniauth_builder.instance_eval(&block)
     }
 
-    allow(OpenProject::Plugins::AuthPlugin).to receive(:strategies).and_return(strategies)
-    allow(MockEngine).to receive(:engine_name).and_return('foobar')
-    allow(MockEngine).to receive(:initializer) { |_, &block| app.instance_eval(&block) }
+    allow(described_class).to receive(:strategies).and_return(strategies)
+    allow(dummy_engine_klass).to receive(:engine_name).and_return('foobar')
+    allow(dummy_engine_klass).to receive(:initializer) { |_, &block| app.instance_eval(&block) }
   end
 
   describe 'ProviderBuilder' do
@@ -70,16 +70,22 @@ describe OpenProject::Plugins::AuthPlugin do
       pb = providers_b.call
       pc = providers_c.call
 
-      Class.new(MockEngine) do
+      Class.new(dummy_engine_klass) do
         register_auth_providers do
-          strategy :strategy_a do; pa; end
-          strategy :strategy_b do; pb; end
+          strategy :strategy_a do
+            pa
+          end
+          strategy :strategy_b do
+            pb
+          end
         end
       end
 
-      Class.new(MockEngine) do
+      Class.new(dummy_engine_klass) do
         register_auth_providers do
-          strategy :strategy_a do; pc; end
+          strategy :strategy_a do
+            pc
+          end
         end
       end
     end
@@ -94,8 +100,9 @@ describe OpenProject::Plugins::AuthPlugin do
     end
 
     it 'associates the correct providers with their respective strategies' do
-      expect(OpenProject::Plugins::AuthPlugin.providers_for(:strategy_a)).to eq [providers_a.call, providers_c.call].flatten
-      expect(OpenProject::Plugins::AuthPlugin.providers_for(:strategy_b)).to eq providers_b.call
+      described_class.providers_for(:strategy_a)
+      expect(described_class.providers_for(:strategy_a)).to eq [providers_a.call, providers_c.call].flatten
+      expect(described_class.providers_for(:strategy_b)).to eq providers_b.call
     end
   end
 end

modules/two_factor_authentication/spec/features/backup_codes/generate_backup_codes_spec.rb

@@ -22,7 +22,7 @@ describe 'Generate 2FA backup codes', with_2fa_ee: true, type: :feature,
 
     # Log token for next access
     backup_codes = nil
-    expect(::TwoFactorAuthentication::BackupCode)
+    allow(::TwoFactorAuthentication::BackupCode)
         .to receive(:regenerate!)
         .and_wrap_original do |m, user|
       backup_codes = m.call(user)
@@ -33,7 +33,7 @@ describe 'Generate 2FA backup codes', with_2fa_ee: true, type: :feature,
     click_on I18n.t('two_factor_authentication.backup_codes.generate.title')
     dialog.confirm_flow_with 'wrong_password', should_fail: true
 
-    # Confirm with correc password
+    # Confirm with correct password
     expect(page).to have_selector('h2', text: I18n.t('two_factor_authentication.backup_codes.plural'))
     click_on I18n.t('two_factor_authentication.backup_codes.generate.title')
     dialog.confirm_flow_with user_password, should_fail: false

spec/controllers/enumerations_controller.rb

@@ -1,82 +0,0 @@
-#-- copyright
-# OpenProject is an open source project management software.
-# Copyright (C) 2012-2022 the OpenProject GmbH
-#
-# This program is free software; you can redistribute it and/or
-# modify it under the terms of the GNU General Public License version 3.
-#
-# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
-# Copyright (C) 2006-2013 Jean-Philippe Lang
-# Copyright (C) 2010-2013 the ChiliProject Team
-#
-# This program is free software; you can redistribute it and/or
-# modify it under the terms of the GNU General Public License
-# as published by the Free Software Foundation; either version 2
-# of the License, or (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program; if not, write to the Free Software
-# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
-#
-# See COPYRIGHT and LICENSE files for more details.
-#++
-
-require 'spec_helper'
-
-describe EnumerationsController, type: :controller do
-  before { allow(controller).to receive(:require_admin).and_return(true) }
-
-  describe '#destroy' do
-    describe '#priority' do
-      let(:enum_to_delete) { create(:priority_normal) }
-
-      shared_examples_for 'successful delete' do
-        it { expect(Enumeration.find_by(id: enum_to_delete.id)).to be_nil }
-
-        it { expect(response).to redirect_to(enumerations_path) }
-      end
-
-      describe 'not in use' do
-        before do
-          post :destroy, params: { id: enum_to_delete.id }
-        end
-
-        it_behaves_like 'successful delete'
-      end
-
-      describe 'in use' do
-        let!(:enum_to_reassign) { create(:priority_high) }
-        let!(:work_package) do
-          create(:work_package,
-                 priority: enum_to_delete)
-        end
-
-        describe 'no reassign' do
-          before do
-            post :destroy, params: { id: enum_to_delete.id }
-          end
-
-          it { expect(assigns(:enumerations)).to include(enum_to_reassign) }
-
-          it { expect(Enumeration.find_by(id: enum_to_delete.id)).not_to be_nil }
-
-          it { expect(response).to render_template('enumerations/destroy') }
-        end
-
-        describe 'reassign' do
-          before do
-            post :destroy,
-                 params: { id: enum_to_delete.id, reassign_to_id: enum_to_reassign.id }
-          end
-
-          it_behaves_like 'successful delete'
-        end
-      end
-    end
-  end
-end

spec/features/work_packages/select/select_card_view_spec.rb

@@ -34,74 +34,74 @@ describe 'Selecting cards in the card view (regression #31962)', js: true do
   let(:wp_table) { Pages::WorkPackagesTable.new(project) }
   let(:cards) { ::Pages::WorkPackageCards.new(project) }
   let(:display_representation) { ::Components::WorkPackages::DisplayRepresentation.new }
-  let(:work_package_1) { create(:work_package, project:) }
-  let(:work_package_2) { create(:work_package, project:) }
-  let(:work_package_3) { create(:work_package, project:) }
+  let!(:work_package1) { create(:work_package, project:) }
+  let!(:work_package2) { create(:work_package, project:) }
+  let!(:work_package3) { create(:work_package, project:) }
 
   before do
-    work_package_1
-    work_package_2
-    work_package_3
+    work_package1
+    work_package2
+    work_package3
 
     login_as(user)
     wp_table.visit!
     display_representation.switch_to_card_layout
-    cards.expect_work_package_listed work_package_1, work_package_2, work_package_3
+    cards.expect_work_package_listed work_package1, work_package2, work_package3
   end
 
-  context 'selecting cards' do
+  describe 'selecting cards' do
     it 'can select and deselect all cards' do
       # Select all
       cards.select_all_work_packages
-      cards.expect_work_package_selected work_package_1, true
-      cards.expect_work_package_selected work_package_2, true
-      cards.expect_work_package_selected work_package_3, true
+      cards.expect_work_package_selected work_package1, true
+      cards.expect_work_package_selected work_package2, true
+      cards.expect_work_package_selected work_package3, true
 
       # Deselect all
       cards.deselect_all_work_packages
-      cards.expect_work_package_selected work_package_1, false
-      cards.expect_work_package_selected work_package_2, false
-      cards.expect_work_package_selected work_package_3, false
+      cards.expect_work_package_selected work_package1, false
+      cards.expect_work_package_selected work_package2, false
+      cards.expect_work_package_selected work_package3, false
     end
 
     it 'can select and deselect single cards' do
       # Select a card
-      cards.select_work_package work_package_1
-      cards.expect_work_package_selected work_package_1, true
-      cards.expect_work_package_selected work_package_2, false
-      cards.expect_work_package_selected work_package_3, false
+      cards.select_work_package work_package1
+      cards.expect_work_package_selected work_package1, true
+      cards.expect_work_package_selected work_package2, false
+      cards.expect_work_package_selected work_package3, false
 
       # Selecting another card changes the selection
-      cards.select_work_package work_package_2
-      cards.expect_work_package_selected work_package_1, false
-      cards.expect_work_package_selected work_package_2, true
-      cards.expect_work_package_selected work_package_3, false
+      cards.select_work_package work_package2
+      cards.expect_work_package_selected work_package1, false
+      cards.expect_work_package_selected work_package2, true
+      cards.expect_work_package_selected work_package3, false
 
       # Deselect a card
-      cards.deselect_work_package work_package_2
-      cards.expect_work_package_selected work_package_1, false
-      cards.expect_work_package_selected work_package_2, false
-      cards.expect_work_package_selected work_package_3, false
+      cards.deselect_work_package work_package2
+      cards.expect_work_package_selected work_package1, false
+      cards.expect_work_package_selected work_package2, false
+      cards.expect_work_package_selected work_package3, false
     end
 
     it 'can select and deselect range of cards' do
       # Select the first WP
-      cards.select_work_package work_package_1
-      cards.expect_work_package_selected work_package_1, true
-      cards.expect_work_package_selected work_package_2, false
-      cards.expect_work_package_selected work_package_3, false
+      cards.select_work_package work_package1
+      cards.expect_work_package_selected work_package1, true
+      cards.expect_work_package_selected work_package2, false
+      cards.expect_work_package_selected work_package3, false
 
       # Select the third with Shift results in all WPs being selected
-      cards.select_work_package_with_shift work_package_3
-      cards.expect_work_package_selected work_package_1, true
-      cards.expect_work_package_selected work_package_2, true
-      cards.expect_work_package_selected work_package_3, true
+      cards.select_work_package_with_shift work_package3
+      cards.expect_work_package_selected work_package1, true
+      cards.expect_work_package_selected work_package2, true
+      cards.expect_work_package_selected work_package3, true
 
       # The range can be changed
-      cards.select_work_package_with_shift work_package_2
-      cards.expect_work_package_selected work_package_1, true
-      cards.expect_work_package_selected work_package_2, true
-      cards.expect_work_package_selected work_package_3, false
+      cards.select_work_package_with_shift work_package2
+      cards.expect_work_package_selected work_package1, true
+      cards.expect_work_package_selected work_package2, true
+      cards.expect_work_package_selected work_package3, false
     end
   end
 end

spec/services/update_projects_types_service_spec.rb

@@ -28,23 +28,26 @@
 require 'spec_helper'
 
 describe UpdateProjectsTypesService do
-  let(:project) { double(Project, types_used_by_work_packages: []) }
-  let(:type) { double(Type, id: 456, name: 'A type') }
-  let(:standard_type) { double('StandardType', id: 123) }
-  let(:instance) { described_class.new(project) }
+  let(:project) { instance_double(Project, types_used_by_work_packages: []) }
+  let(:standard_type) { build_stubbed(:type_standard) }
+
+  subject(:instance) { described_class.new(project) }
 
   before do
     allow(Type).to receive(:standard_type).and_return standard_type
   end
 
   describe '.call' do
+    before do
+      allow(project).to receive(:type_ids=)
+    end
+
     context 'with ids provided' do
       let(:ids) { [1, 2, 3] }
 
       it 'returns true and updates the ids' do
-        expect(project).to receive(:type_ids=).with(ids)
-
         expect(instance.call(ids)).to be_truthy
+        expect(project).to have_received(:type_ids=).with(ids)
       end
     end
 
@@ -52,9 +55,8 @@ describe UpdateProjectsTypesService do
       let(:ids) { [] }
 
       it 'adds the id of the default type and returns true' do
-        expect(project).to receive(:type_ids=).with([standard_type.id])
-
         expect(instance.call(ids)).to be_truthy
+        expect(project).to have_received(:type_ids=).with([standard_type.id])
       end
     end
 
@@ -62,13 +64,14 @@ describe UpdateProjectsTypesService do
       let(:ids) { nil }
 
       it 'adds the id of the default type and returns true' do
-        expect(project).to receive(:type_ids=).with([standard_type.id])
-
         expect(instance.call(ids)).to be_truthy
+        expect(project).to have_received(:type_ids=).with([standard_type.id])
       end
     end
 
-    context 'the id of a type in use is not provided' do
+    context 'when the id of a type in use is not provided' do
+      let(:type) { build_stubbed(:type) }
+
       before do
         allow(project).to receive(:types_used_by_work_packages).and_return([type])
       end
@@ -76,13 +79,13 @@ describe UpdateProjectsTypesService do
       it 'returns false and sets an error message' do
         ids = [1]
 
-        errors = double('Errors')
-        expect(project).to receive(:errors).and_return(errors)
-        expect(errors).to receive(:add).with(:type, :in_use_by_work_packages, types: type.name)
-
-        expect(project).not_to receive(:type_ids=)
+        errors = instance_double(ActiveModel::Errors)
+        allow(errors).to receive(:add)
+        allow(project).to receive(:errors).and_return(errors)
 
         expect(instance.call(ids)).to be_falsey
+        expect(errors).to have_received(:add).with(:types, :in_use_by_work_packages, types: type.name)
+        expect(project).not_to have_received(:type_ids=)
       end
     end
   end

spec/tasks/backup_spec.rb

@@ -28,47 +28,49 @@
 
 require 'spec_helper'
 
-describe 'postgresql' do
+RSpec.describe Rake::Task, 'backup:database' do
   let(:database_config) do
     { 'adapter' => 'postgresql',
       'database' => 'openproject-database',
-      'username' => 'testuser',
-      'password' => 'testpassword' }
+      'username' => 'test_user',
+      'password' => 'test_password' }
   end
 
   before do
-    expect(ActiveRecord::Base).to receive(:configurations).at_least(:once).and_return('test' => database_config)
+    allow(ActiveRecord::Base).to receive(:configurations).and_return('test' => database_config)
     allow(FileUtils).to receive(:mkdir_p).and_return(nil)
+    allow(Kernel).to receive(:system)
   end
 
   describe 'backup:database:create' do
     include_context 'rake'
 
     it 'calls the pg_dump binary' do
-      expect(Kernel).to receive(:system) do |*args|
+      subject.invoke
+      expect(Kernel).to have_received(:system) do |*args|
         expect(args[1]).to eql('pg_dump')
       end
-      subject.invoke
     end
 
     it 'writes the pg password file' do
-      expect(Kernel).to receive(:system) do |*args|
+      # can't use have_received because password file is deleted after invocation
+      expect(Kernel).to receive(:system) do |*args| # rubocop:disable RSpec/MessageSpies
         pass_file = args.first['PGPASSFILE']
         expect(File.readable?(pass_file)).to be true
 
         file_contents = File.read pass_file
-        expect(file_contents).to include('testpassword')
+        expect(file_contents).to include('test_password')
       end
       subject.invoke
     end
 
     it 'uses the first task parameter as the target filename' do
-      custom_file_path = './foo/bar/testfile.sql'
-      expect(Kernel).to receive(:system) do |*args|
+      custom_file_path = './foo/bar/test_file.sql'
+      subject.invoke custom_file_path
+      expect(Kernel).to have_received(:system) do |*args|
         result_file = args.find { |s| s.to_s.starts_with? '--file=' }
         expect(result_file).to include(custom_file_path)
       end
-      subject.invoke custom_file_path
     end
   end
 
@@ -84,32 +86,33 @@ describe 'postgresql' do
     end
 
     it 'calls the pg_restore binary' do
-      expect(Kernel).to receive(:system) do |*args|
+      subject.invoke backup_file.path
+      expect(Kernel).to have_received(:system) do |*args|
         expect(args[1]).to start_with('pg_restore')
       end
-      subject.invoke backup_file.path
     end
 
     it 'writes the pg password file' do
-      expect(Kernel).to receive(:system) do |*args|
+      # can't use have_received because password file is deleted after invocation
+      expect(Kernel).to receive(:system) do |*args| # rubocop:disable RSpec/MessageSpies
         pass_file = args.first['PGPASSFILE']
         expect(File.readable?(pass_file)).to be true
 
         file_contents = File.read pass_file
-        expect(file_contents).to include('testpassword')
+        expect(file_contents).to include('test_password')
       end
       subject.invoke backup_file.path
     end
 
     it 'uses the first task parameter as the target filename' do
-      expect(Kernel).to receive(:system) do |*args|
+      subject.invoke backup_file.path
+      expect(Kernel).to have_received(:system) do |*args|
         expect(args.last).to eql(backup_file.path)
       end
-      subject.invoke backup_file.path
     end
 
     it 'throws an error when called without a parameter' do
-      expect { subject.invoke }.to raise_error
+      expect { subject.invoke }.to raise_error(RuntimeError, "You must provide the path to the database dump")
     end
   end
 end

spec/workers/mails/shared/member_job.rb

@@ -97,7 +97,7 @@ shared_examples 'member job' do
     %i[added_project updated_global updated_project].each do |mails|
       allow(MemberMailer)
         .to receive(mails)
-              .and_return(double('mail', deliver_now: nil))  # rubocop:disable Rspec/VerifiedDoubles
+              .and_return(double('mail', deliver_now: nil))  # rubocop:disable RSpec/VerifiedDoubles
     end
   end