Speed-up in `Settings::` controllers specs (#27808)

spec/controllers/settings/aliases_controller_spec.rb

@@ -17,11 +17,8 @@ describe Settings::AliasesController do
       get :index
     end
 
-    it 'returns http success' do
+    it 'returns http success with private cache control headers', :aggregate_failures do
       expect(response).to have_http_status(200)
-    end
-
-    it 'returns private cache control headers' do
       expect(response.headers['Cache-Control']).to include('private, no-store')
     end
   end

spec/controllers/settings/applications_controller_spec.rb

@@ -18,11 +18,8 @@ describe Settings::ApplicationsController do
       get :index
     end
 
-    it 'returns http success' do
+    it 'returns http success with private cache control headers', :aggregate_failures do
       expect(response).to have_http_status(200)
-    end
-
-    it 'returns private cache control headers' do
       expect(response.headers['Cache-Control']).to include('private, no-store')
     end
   end
@@ -51,7 +48,7 @@ describe Settings::ApplicationsController do
 
   describe 'POST #create' do
     context 'when success (passed scopes as a String)' do
-      def call_create
+      subject do
         post :create, params: {
           doorkeeper_application: {
             name: 'My New App',
@@ -60,20 +57,16 @@ describe Settings::ApplicationsController do
             scopes: 'read write follow',
           },
         }
-        response
       end
 
-      it 'creates an entry in the database' do
-        expect { call_create }.to change(Doorkeeper::Application, :count)
-      end
-
-      it 'redirects back to applications page' do
-        expect(call_create).to redirect_to(settings_applications_path)
+      it 'creates an entry in the database', :aggregate_failures do
+        expect { subject }.to change(Doorkeeper::Application, :count)
+        expect(response).to redirect_to(settings_applications_path)
       end
     end
 
     context 'when success (passed scopes as an Array)' do
-      def call_create
+      subject do
         post :create, params: {
           doorkeeper_application: {
             name: 'My New App',
@@ -82,15 +75,11 @@ describe Settings::ApplicationsController do
             scopes: %w(read write follow),
           },
         }
-        response
       end
 
-      it 'creates an entry in the database' do
-        expect { call_create }.to change(Doorkeeper::Application, :count)
-      end
-
-      it 'redirects back to applications page' do
-        expect(call_create).to redirect_to(settings_applications_path)
+      it 'creates an entry in the database', :aggregate_failures do
+        expect { subject }.to change(Doorkeeper::Application, :count)
+        expect(response).to redirect_to(settings_applications_path)
       end
     end
 
@@ -106,11 +95,8 @@ describe Settings::ApplicationsController do
         }
       end
 
-      it 'returns http success' do
+      it 'returns http success and renders form', :aggregate_failures do
         expect(response).to have_http_status(200)
-      end
-
-      it 'renders form again' do
         expect(response).to render_template(:new)
       end
     end
@@ -118,13 +104,7 @@ describe Settings::ApplicationsController do
 
   describe 'PATCH #update' do
     context 'when success' do
-      let(:opts) do
-        {
-          website: 'https://foo.bar/',
-        }
-      end
-
-      def call_update
+      subject do
         patch :update, params: {
           id: app.id,
           doorkeeper_application: opts,
@@ -132,13 +112,17 @@ describe Settings::ApplicationsController do
         response
       end
 
-      it 'updates existing application' do
-        call_update
-        expect(app.reload.website).to eql(opts[:website])
+      let(:opts) do
+        {
+          website: 'https://foo.bar/',
+        }
       end
 
-      it 'redirects back to applications page' do
-        expect(call_update).to redirect_to(settings_application_path(app))
+      it 'updates existing application' do
+        subject
+
+        expect(app.reload.website).to eql(opts[:website])
+        expect(response).to redirect_to(settings_application_path(app))
       end
     end
 
@@ -155,11 +139,8 @@ describe Settings::ApplicationsController do
         }
       end
 
-      it 'returns http success' do
+      it 'returns http success and renders form', :aggregate_failures do
         expect(response).to have_http_status(200)
-      end
-
-      it 'renders form again' do
         expect(response).to render_template(:show)
       end
     end
@@ -170,11 +151,8 @@ describe Settings::ApplicationsController do
       post :destroy, params: { id: app.id }
     end
 
-    it 'redirects back to applications page' do
+    it 'redirects back to applications page and removes the app' do
       expect(response).to redirect_to(settings_applications_path)
-    end
-
-    it 'removes the app' do
       expect(Doorkeeper::Application.find_by(id: app.id)).to be_nil
     end
   end

spec/controllers/settings/deletes_controller_spec.rb

@@ -14,22 +14,16 @@ describe Settings::DeletesController do
         get :show
       end
 
-      it 'renders confirmation page' do
+      it 'renders confirmation page with private cache control headers', :aggregate_failures do
         expect(response).to have_http_status(200)
-      end
-
-      it 'returns private cache control headers' do
         expect(response.headers['Cache-Control']).to include('private, no-store')
       end
 
       context 'when suspended' do
         let(:user) { Fabricate(:user, account_attributes: { suspended_at: Time.now.utc }) }
 
-        it 'returns http forbidden' do
+        it 'returns http forbidden with private cache control headers', :aggregate_failures do
           expect(response).to have_http_status(403)
-        end
-
-        it 'returns private cache control headers' do
           expect(response.headers['Cache-Control']).to include('private, no-store')
         end
       end
@@ -56,19 +50,10 @@ describe Settings::DeletesController do
           delete :destroy, params: { form_delete_confirmation: { password: 'petsmoldoggos' } }
         end
 
-        it 'redirects to sign in page' do
+        it 'removes user record and redirects', :aggregate_failures do
           expect(response).to redirect_to '/auth/sign_in'
-        end
-
-        it 'removes user record' do
           expect(User.find_by(id: user.id)).to be_nil
-        end
-
-        it 'marks account as suspended' do
           expect(user.account.reload).to be_suspended
-        end
-
-        it 'does not create an email block' do
           expect(CanonicalEmailBlock.block?(user.email)).to be false
         end
 

spec/controllers/settings/exports_controller_spec.rb

@@ -14,11 +14,8 @@ describe Settings::ExportsController do
         get :show
       end
 
-      it 'returns http success' do
+      it 'returns http success with private cache control headers', :aggregate_failures do
         expect(response).to have_http_status(200)
-      end
-
-      it 'returns private cache control headers' do
         expect(response.headers['Cache-Control']).to include('private, no-store')
       end
     end

spec/controllers/settings/imports_controller_spec.rb

@@ -19,15 +19,9 @@ RSpec.describe Settings::ImportsController do
       get :index
     end
 
-    it 'assigns the expected imports' do
-      expect(assigns(:recent_imports)).to eq [import]
-    end
-
-    it 'returns http success' do
+    it 'assigns the expected imports', :aggregate_failures do
       expect(response).to have_http_status(200)
-    end
-
-    it 'returns private cache control headers' do
+      expect(assigns(:recent_imports)).to eq [import]
       expect(response.headers['Cache-Control']).to include('private, no-store')
     end
   end
@@ -72,17 +66,10 @@ RSpec.describe Settings::ImportsController do
     context 'with someone else\'s import' do
       let(:bulk_import) { Fabricate(:bulk_import, state: :unconfirmed) }
 
-      it 'does not change the import\'s state' do
+      it 'does not change the import\'s state and returns missing', :aggregate_failures do
         expect { subject }.to_not(change { bulk_import.reload.state })
-      end
 
-      it 'does not fire the import worker' do
-        subject
         expect(BulkImportWorker).to_not have_received(:perform_async)
-      end
-
-      it 'returns http not found' do
-        subject
         expect(response).to have_http_status(404)
       end
     end
@@ -90,17 +77,10 @@ RSpec.describe Settings::ImportsController do
     context 'with an already-confirmed import' do
       let(:bulk_import) { Fabricate(:bulk_import, account: user.account, state: :in_progress) }
 
-      it 'does not change the import\'s state' do
+      it 'does not change the import\'s state and returns missing', :aggregate_failures do
         expect { subject }.to_not(change { bulk_import.reload.state })
-      end
 
-      it 'does not fire the import worker' do
-        subject
         expect(BulkImportWorker).to_not have_received(:perform_async)
-      end
-
-      it 'returns http not found' do
-        subject
         expect(response).to have_http_status(404)
       end
     end
@@ -108,17 +88,10 @@ RSpec.describe Settings::ImportsController do
     context 'with an unconfirmed import' do
       let(:bulk_import) { Fabricate(:bulk_import, account: user.account, state: :unconfirmed) }
 
-      it 'changes the import\'s state to scheduled' do
+      it 'changes the import\'s state to scheduled and redirects', :aggregate_failures do
         expect { subject }.to change { bulk_import.reload.state.to_sym }.from(:unconfirmed).to(:scheduled)
-      end
 
-      it 'fires the import worker on the expected import' do
-        subject
         expect(BulkImportWorker).to have_received(:perform_async).with(bulk_import.id)
-      end
-
-      it 'redirects to imports path' do
-        subject
         expect(response).to redirect_to(settings_imports_path)
       end
     end
@@ -130,12 +103,9 @@ RSpec.describe Settings::ImportsController do
     context 'with someone else\'s import' do
       let(:bulk_import) { Fabricate(:bulk_import, state: :unconfirmed) }
 
-      it 'does not delete the import' do
+      it 'does not delete the import and returns missing', :aggregate_failures do
         expect { subject }.to_not(change { BulkImport.exists?(bulk_import.id) })
-      end
 
-      it 'returns http not found' do
-        subject
         expect(response).to have_http_status(404)
       end
     end
@@ -143,12 +113,9 @@ RSpec.describe Settings::ImportsController do
     context 'with an already-confirmed import' do
       let(:bulk_import) { Fabricate(:bulk_import, account: user.account, state: :in_progress) }
 
-      it 'does not delete the import' do
+      it 'does not delete the import and returns missing', :aggregate_failures do
         expect { subject }.to_not(change { BulkImport.exists?(bulk_import.id) })
-      end
 
-      it 'returns http not found' do
-        subject
         expect(response).to have_http_status(404)
       end
     end
@@ -156,12 +123,9 @@ RSpec.describe Settings::ImportsController do
     context 'with an unconfirmed import' do
       let(:bulk_import) { Fabricate(:bulk_import, account: user.account, state: :unconfirmed) }
 
-      it 'deletes the import' do
+      it 'deletes the import and redirects', :aggregate_failures do
         expect { subject }.to change { BulkImport.exists?(bulk_import.id) }.from(true).to(false)
-      end
 
-      it 'redirects to imports path' do
-        subject
         expect(response).to redirect_to(settings_imports_path)
       end
     end
@@ -177,13 +141,10 @@ RSpec.describe Settings::ImportsController do
         bulk_import.update(total_items: bulk_import.rows.count, processed_items: bulk_import.rows.count, imported_items: 0)
       end
 
-      it 'returns http success' do
+      it 'returns expected contents', :aggregate_failures do
         subject
-        expect(response).to have_http_status(200)
-      end
 
-      it 'returns expected contents' do
-        subject
+        expect(response).to have_http_status(200)
         expect(response.body).to eq expected_contents
       end
     end
@@ -283,12 +244,9 @@ RSpec.describe Settings::ImportsController do
       let(:import_file) { file }
       let(:import_mode) { mode }
 
-      it 'creates an unconfirmed bulk_import with expected type' do
+      it 'creates an unconfirmed bulk_import with expected type and redirects', :aggregate_failures do
         expect { subject }.to change { user.account.bulk_imports.pluck(:state, :type) }.from([]).to([['unconfirmed', import_type]])
-      end
 
-      it 'redirects to confirmation page for the import' do
-        subject
         expect(response).to redirect_to(settings_import_path(user.account.bulk_imports.first))
       end
     end
@@ -298,12 +256,9 @@ RSpec.describe Settings::ImportsController do
       let(:import_file) { file }
       let(:import_mode) { mode }
 
-      it 'does not creates an unconfirmed bulk_import' do
+      it 'does not creates an unconfirmed bulk_import', :aggregate_failures do
         expect { subject }.to_not(change { user.account.bulk_imports.count })
-      end
 
-      it 'sets error to the import' do
-        subject
         expect(assigns(:import).errors).to_not be_empty
       end
     end

spec/controllers/settings/login_activities_controller_spec.rb

@@ -16,11 +16,8 @@ describe Settings::LoginActivitiesController do
       get :index
     end
 
-    it 'returns http success' do
+    it 'returns http success with private cache control headers', :aggregate_failures do
       expect(response).to have_http_status(200)
-    end
-
-    it 'returns private cache control headers' do
       expect(response.headers['Cache-Control']).to include('private, no-store')
     end
   end

spec/controllers/settings/migration/redirects_controller_spec.rb

@@ -16,11 +16,8 @@ describe Settings::Migration::RedirectsController do
       get :new
     end
 
-    it 'returns http success' do
+    it 'returns http success with private cache control headers', :aggregate_failures do
       expect(response).to have_http_status(200)
-    end
-
-    it 'returns private cache control headers' do
       expect(response.headers['Cache-Control']).to include('private, no-store')
     end
   end

spec/controllers/settings/migrations_controller_spec.rb

@@ -71,24 +71,22 @@ describe Settings::MigrationsController do
       context 'when acct is the current account' do
         let(:acct) { user.account }
 
-        it 'renders show' do
-          expect(subject).to render_template :show
-        end
+        it 'does not update the moved account', :aggregate_failures do
+          subject
 
-        it 'does not update the moved account' do
           expect(user.account.reload.moved_to_account_id).to be_nil
+          expect(response).to render_template :show
         end
       end
 
       context 'when target account does not reference the account being moved from' do
         let(:acct) { Fabricate(:account, also_known_as: []) }
 
-        it 'renders show' do
-          expect(subject).to render_template :show
-        end
+        it 'does not update the moved account', :aggregate_failures do
+          subject
 
-        it 'does not update the moved account' do
           expect(user.account.reload.moved_to_account_id).to be_nil
+          expect(response).to render_template :show
         end
       end
 
@@ -100,12 +98,11 @@ describe Settings::MigrationsController do
           user.account.migrations.create!(acct: moved_to.acct)
         end
 
-        it 'renders show' do
-          expect(subject).to render_template :show
-        end
+        it 'does not update the moved account', :aggregate_failures do
+          subject
 
-        it 'does not update the moved account' do
           expect(user.account.reload.moved_to_account_id).to be_nil
+          expect(response).to render_template :show
         end
       end
     end

spec/controllers/settings/preferences/appearance_controller_spec.rb

@@ -16,11 +16,8 @@ describe Settings::Preferences::AppearanceController do
       get :show
     end
 
-    it 'returns http success' do
+    it 'returns http success with private cache control headers', :aggregate_failures do
       expect(response).to have_http_status(200)
-    end
-
-    it 'returns private cache control headers' do
       expect(response.headers['Cache-Control']).to include('private, no-store')
     end
   end

spec/controllers/settings/preferences/notifications_controller_spec.rb

@@ -16,11 +16,8 @@ describe Settings::Preferences::NotificationsController do
       get :show
     end
 
-    it 'returns http success' do
+    it 'returns http success with private cache control headers', :aggregate_failures do
       expect(response).to have_http_status(200)
-    end
-
-    it 'returns private cache control headers' do
       expect(response.headers['Cache-Control']).to include('private, no-store')
     end
   end

spec/controllers/settings/preferences/other_controller_spec.rb

@@ -16,11 +16,8 @@ describe Settings::Preferences::OtherController do
       get :show
     end
 
-    it 'returns http success' do
+    it 'returns http success with private cache control headers', :aggregate_failures do
       expect(response).to have_http_status(200)
-    end
-
-    it 'returns private cache control headers' do
       expect(response.headers['Cache-Control']).to include('private, no-store')
     end
   end

spec/controllers/settings/profiles_controller_spec.rb

@@ -17,11 +17,8 @@ RSpec.describe Settings::ProfilesController do
       get :show
     end
 
-    it 'returns http success' do
+    it 'returns http success with private cache control headers', :aggregate_failures do
       expect(response).to have_http_status(200)
-    end
-
-    it 'returns private cache control headers' do
       expect(response.headers['Cache-Control']).to include('private, no-store')
     end
   end

spec/controllers/settings/two_factor_authentication/webauthn_credentials_controller_spec.rb

@@ -121,24 +121,12 @@ describe Settings::TwoFactorAuthentication::WebauthnCredentialsController do
             add_webauthn_credential(user)
           end
 
-          it 'returns http success' do
-            get :options
+          it 'includes existing credentials in list of excluded credentials', :aggregate_failures do
+            expect { get :options }.to_not change(user, :webauthn_id)
 
             expect(response).to have_http_status(200)
-          end
-
-          it 'stores the challenge on the session' do
-            get :options
 
             expect(controller.session[:webauthn_challenge]).to be_present
-          end
-
-          it 'does not change webauthn_id' do
-            expect { get :options }.to_not change(user, :webauthn_id)
-          end
-
-          it 'includes existing credentials in list of excluded credentials' do
-            get :options
 
             excluded_credentials_ids = response.parsed_body['excludeCredentials'].pluck('id')
             expect(excluded_credentials_ids).to match_array(user.webauthn_credentials.pluck(:external_id))
@@ -146,21 +134,11 @@ describe Settings::TwoFactorAuthentication::WebauthnCredentialsController do
         end
 
         context 'when user does not have webauthn enabled' do
-          it 'returns http success' do
+          it 'stores the challenge on the session and sets user webauthn_id', :aggregate_failures do
             get :options
 
             expect(response).to have_http_status(200)
-          end
-
-          it 'stores the challenge on the session' do
-            get :options
-
             expect(controller.session[:webauthn_challenge]).to be_present
-          end
-
-          it 'sets user webauthn_id' do
-            get :options
-
             expect(user.reload.webauthn_id).to be_present
           end
         end
@@ -217,28 +195,15 @@ describe Settings::TwoFactorAuthentication::WebauthnCredentialsController do
           end
 
           context 'when creation succeeds' do
-            it 'returns http success' do
-              controller.session[:webauthn_challenge] = challenge
-
-              post :create, params: { credential: new_webauthn_credential, nickname: nickname }
-
-              expect(response).to have_http_status(200)
-            end
-
-            it 'adds a new credential to user credentials' do
+            it 'adds a new credential to user credentials and does not change webauthn_id', :aggregate_failures do
               controller.session[:webauthn_challenge] = challenge
 
               expect do
                 post :create, params: { credential: new_webauthn_credential, nickname: nickname }
               end.to change { user.webauthn_credentials.count }.by(1)
-            end
-
-            it 'does not change webauthn_id' do
-              controller.session[:webauthn_challenge] = challenge
+                                                               .and not_change(user, :webauthn_id)
 
-              expect do
-                post :create, params: { credential: new_webauthn_credential, nickname: nickname }
-              end.to_not change(user, :webauthn_id)
+              expect(response).to have_http_status(200)
             end
           end
 
@@ -328,18 +293,14 @@ describe Settings::TwoFactorAuthentication::WebauthnCredentialsController do
           end
 
           context 'when deletion succeeds' do
-            it 'redirects to 2FA methods list and shows flash success' do
+            it 'redirects to 2FA methods list and shows flash success and deletes the credential', :aggregate_failures do
+              expect do
                 delete :destroy, params: { id: user.webauthn_credentials.take.id }
+              end.to change { user.webauthn_credentials.count }.by(-1)
 
               expect(response).to redirect_to settings_two_factor_authentication_methods_path
               expect(flash[:success]).to be_present
             end
-
-            it 'deletes the credential' do
-              expect do
-                delete :destroy, params: { id: user.webauthn_credentials.take.id }
-              end.to change { user.webauthn_credentials.count }.by(-1)
-            end
           end
         end
 

spec/controllers/settings/two_factor_authentication_methods_controller_spec.rb

@@ -29,11 +29,8 @@ describe Settings::TwoFactorAuthenticationMethodsController do
           get :index
         end
 
-        it 'returns http success' do
+        it 'returns http success with private cache control headers', :aggregate_failures do
           expect(response).to have_http_status(200)
-        end
-
-        it 'returns private cache control headers' do
           expect(response.headers['Cache-Control']).to include('private, no-store')
         end
       end