Merge remote-tracking branch 'parent/main' into upstream-20240517

spec/controllers/admin/account_moderation_notes_controller_spec.rb

@@ -24,10 +24,19 @@ RSpec.describe Admin::AccountModerationNotesController do
       end
     end
 
-    context 'when parameters are invalid' do
+    context 'when the content is too short' do
       let(:params) { { account_moderation_note: { target_account_id: target_account.id, content: '' } } }
 
-      it 'falls to create a note' do
+      it 'fails to create a note' do
+        expect { subject }.to_not change(AccountModerationNote, :count)
+        expect(response).to render_template 'admin/accounts/show'
+      end
+    end
+
+    context 'when the content is too long' do
+      let(:params) { { account_moderation_note: { target_account_id: target_account.id, content: 'test' * AccountModerationNote::CONTENT_SIZE_LIMIT } } }
+
+      it 'fails to create a note' do
         expect { subject }.to_not change(AccountModerationNote, :count)
         expect(response).to render_template 'admin/accounts/show'
       end

spec/controllers/admin/report_notes_controller_spec.rb

@@ -22,7 +22,7 @@ describe Admin::ReportNotesController do
         let(:account_id) { nil }
 
         context 'when create_and_resolve flag is on' do
-          let(:params) { { report_note: { content: 'test content', report_id: report.id }, create_and_resolve: nil } }
+          let(:params) { { report_note: { report_id: report.id, content: 'test content' }, create_and_resolve: nil } }
 
           it 'creates a report note and resolves report' do
             expect { subject }.to change(ReportNote, :count).by(1)
@@ -32,7 +32,7 @@ describe Admin::ReportNotesController do
         end
 
         context 'when create_and_resolve flag is false' do
-          let(:params) { { report_note: { content: 'test content', report_id: report.id } } }
+          let(:params) { { report_note: { report_id: report.id, content: 'test content' } } }
 
           it 'creates a report note and does not resolve report' do
             expect { subject }.to change(ReportNote, :count).by(1)
@@ -47,7 +47,7 @@ describe Admin::ReportNotesController do
         let(:account_id) { user.account.id }
 
         context 'when create_and_unresolve flag is on' do
-          let(:params) { { report_note: { content: 'test content', report_id: report.id }, create_and_unresolve: nil } }
+          let(:params) { { report_note: { report_id: report.id, content: 'test content' }, create_and_unresolve: nil } }
 
           it 'creates a report note and unresolves report' do
             expect { subject }.to change(ReportNote, :count).by(1)
@@ -57,7 +57,7 @@ describe Admin::ReportNotesController do
         end
 
         context 'when create_and_unresolve flag is false' do
-          let(:params) { { report_note: { content: 'test content', report_id: report.id } } }
+          let(:params) { { report_note: { report_id: report.id, content: 'test content' } } }
 
           it 'creates a report note and does not unresolve report' do
             expect { subject }.to change(ReportNote, :count).by(1)
@@ -68,12 +68,24 @@ describe Admin::ReportNotesController do
       end
     end
 
-    context 'when parameter is invalid' do
-      let(:params) { { report_note: { content: '', report_id: report.id } } }
+    context 'when content is too short' do
+      let(:params) { { report_note: { report_id: report.id, content: '' } } }
       let(:action_taken) { nil }
       let(:account_id) { nil }
 
       it 'renders admin/reports/show' do
+        expect { subject }.to_not change(ReportNote, :count)
+        expect(subject).to render_template 'admin/reports/show'
+      end
+    end
+
+    context 'when content is too long' do
+      let(:params) { { report_note: { report_id: report.id, content: 'test' * ReportNote::CONTENT_SIZE_LIMIT } } }
+      let(:action_taken) { nil }
+      let(:account_id) { nil }
+
+      it 'renders admin/reports/show' do
+        expect { subject }.to_not change(ReportNote, :count)
         expect(subject).to render_template 'admin/reports/show'
       end
     end

spec/controllers/concerns/preloading_concern_spec.rb

@@ -2,20 +2,20 @@
 
 require 'rails_helper'
 
-RSpec.describe CacheConcern do
+RSpec.describe PreloadingConcern do
   controller(ApplicationController) do
-    include CacheConcern
+    include PreloadingConcern
 
     def empty_array
-      render plain: cache_collection([], Status).size
+      render plain: preload_collection([], Status).size
     end
 
     def empty_relation
-      render plain: cache_collection(Status.none, Status).size
+      render plain: preload_collection(Status.none, Status).size
     end
 
     def account_statuses_favourites
-      render plain: cache_collection(Status.where(account_id: params[:id]), Status).map(&:favourites_count)
+      render plain: preload_collection(Status.where(account_id: params[:id]), Status).map(&:favourites_count)
     end
   end
 
@@ -27,7 +27,7 @@ RSpec.describe CacheConcern do
     end
   end
 
-  describe '#cache_collection' do
+  describe '#preload_collection' do
     context 'when given an empty array' do
       it 'returns an empty array' do
         get :empty_array

spec/helpers/application_helper_spec.rb

@@ -290,26 +290,31 @@ describe ApplicationHelper do
     end
   end
 
-  describe '#site_icon_path' do
+  describe 'favicon' do
     context 'when an icon exists' do
       let!(:favicon) { Fabricate(:site_upload, var: 'favicon') }
+      let!(:app_icon) { Fabricate(:site_upload, var: 'app_icon') }
 
       it 'returns the URL of the icon' do
-        expect(helper.site_icon_path('favicon')).to eq(favicon.file.url('48'))
+        expect(helper.favicon_path).to eq(favicon.file.url('48'))
+        expect(helper.app_icon_path).to eq(app_icon.file.url('48'))
       end
 
       it 'returns the URL of the icon with size parameter' do
-        expect(helper.site_icon_path('favicon', 16)).to eq(favicon.file.url('16'))
+        expect(helper.favicon_path(16)).to eq(favicon.file.url('16'))
+        expect(helper.app_icon_path(16)).to eq(app_icon.file.url('16'))
       end
     end
 
     context 'when an icon does not exist' do
       it 'returns nil' do
-        expect(helper.site_icon_path('favicon')).to be_nil
+        expect(helper.favicon_path).to be_nil
+        expect(helper.app_icon_path).to be_nil
       end
 
       it 'returns nil with size parameter' do
-        expect(helper.site_icon_path('favicon', 16)).to be_nil
+        expect(helper.favicon_path(16)).to be_nil
+        expect(helper.app_icon_path(16)).to be_nil
       end
     end
   end

spec/rails_helper.rb

@@ -2,14 +2,35 @@
 
 ENV['RAILS_ENV'] ||= 'test'
 
-# This needs to be defined before Rails is initialized
-RUN_SYSTEM_SPECS = ENV.fetch('RUN_SYSTEM_SPECS', false)
+unless ENV['DISABLE_SIMPLECOV'] == 'true'
+  require 'simplecov'
 
-if RUN_SYSTEM_SPECS
-  STREAMING_PORT = ENV.fetch('TEST_STREAMING_PORT', '4020')
-  ENV['STREAMING_API_BASE_URL'] = "http://localhost:#{STREAMING_PORT}"
+  SimpleCov.start 'rails' do
+    if ENV['CI']
+      require 'simplecov-lcov'
+      formatter SimpleCov::Formatter::LcovFormatter
+      formatter.config.report_with_single_file = true
+    else
+      formatter SimpleCov::Formatter::HTMLFormatter
+    end
+
+    enable_coverage :branch
+
+    add_filter 'lib/linter'
+
+    add_group 'Libraries', 'lib'
+    add_group 'Policies', 'app/policies'
+    add_group 'Presenters', 'app/presenters'
+    add_group 'Serializers', 'app/serializers'
+    add_group 'Services', 'app/services'
+    add_group 'Validators', 'app/validators'
+  end
 end
 
+# This needs to be defined before Rails is initialized
+STREAMING_PORT = ENV.fetch('TEST_STREAMING_PORT', '4020')
+ENV['STREAMING_API_BASE_URL'] = "http://localhost:#{STREAMING_PORT}"
+
 require File.expand_path('../config/environment', __dir__)
 
 abort('The Rails environment is running in production mode!') if Rails.env.production?
@@ -26,10 +47,12 @@ require 'test_prof/recipes/rspec/before_all'
 Dir[Rails.root.join('spec', 'support', '**', '*.rb')].each { |f| require f }
 
 ActiveRecord::Migration.maintain_test_schema!
-WebMock.disable_net_connect!(allow: Chewy.settings[:host], allow_localhost: RUN_SYSTEM_SPECS)
+WebMock.disable_net_connect!(
+  allow_localhost: true,
+  allow: Chewy.settings[:host]
+)
 Sidekiq.logger = nil
 
-# System tests config
 DatabaseCleaner.strategy = [:deletion]
 
 Devise::Test::ControllerHelpers.module_eval do
@@ -49,16 +72,14 @@ Devise::Test::ControllerHelpers.module_eval do
 end
 
 RSpec.configure do |config|
-  # This is set before running spec:system, see lib/tasks/tests.rake
-  config.filter_run_excluding type: lambda { |type|
-    case type
-    when :system
-      !RUN_SYSTEM_SPECS
-    end
-  }
+  # By default, skip specs that need full JS browser
+  config.filter_run_excluding :js
 
-  # By default, skip the elastic search integration specs
-  config.filter_run_excluding search: true
+  # By default, skip specs that need elastic search server
+  config.filter_run_excluding :search
+
+  # By default, skip specs that need the streaming server
+  config.filter_run_excluding :streaming
 
   config.fixture_paths = [
     Rails.root.join('spec', 'fixtures'),
@@ -81,7 +102,7 @@ RSpec.configure do |config|
   config.include Devise::Test::ControllerHelpers, type: :controller
   config.include Devise::Test::ControllerHelpers, type: :helper
   config.include Devise::Test::ControllerHelpers, type: :view
-  config.include Devise::Test::IntegrationHelpers, type: :feature
+  config.include Devise::Test::IntegrationHelpers, type: :system
   config.include Devise::Test::IntegrationHelpers, type: :request
   config.include ActionMailer::TestHelper
   config.include Paperclip::Shoulda::Matchers
@@ -111,10 +132,6 @@ RSpec.configure do |config|
     stub_reset_connection_pools
   end
 
-  config.before :each, type: :feature do
-    Capybara.current_driver = :rack_test
-  end
-
   config.before do |example|
     allow(Resolv::DNS).to receive(:open).and_raise('Real DNS queries are disabled, stub Resolv::DNS as needed') unless example.metadata[:type] == :system
   end

spec/requests/content_security_policy_spec.rb

@@ -23,7 +23,7 @@ describe 'Content-Security-Policy' do
     <<~CSP.split("\n").map(&:strip)
       base-uri 'none'
       child-src 'self' blob: https://cb6e6126.ngrok.io
-      connect-src 'self' data: blob: https://cb6e6126.ngrok.io ws://cb6e6126.ngrok.io:4000
+      connect-src 'self' data: blob: https://cb6e6126.ngrok.io #{Rails.configuration.x.streaming_api_base_url}
       default-src 'none'
       font-src 'self' https://cb6e6126.ngrok.io
       form-action 'self'

spec/spec_helper.rb

@@ -1,9 +1,5 @@
 # frozen_string_literal: true
 
-unless ENV['DISABLE_SIMPLECOV'] == 'true'
-  require 'simplecov' # Configuration details loaded from .simplecov
-end
-
 RSpec.configure do |config|
   config.example_status_persistence_file_path = 'tmp/rspec/examples.txt'
   config.expect_with :rspec do |expectations|

spec/support/capybara.rb

@@ -26,6 +26,10 @@ Capybara.javascript_driver = :headless_chrome
 
 RSpec.configure do |config|
   config.before(:each, type: :system) do
+    driven_by :rack_test
+  end
+
+  config.before(:each, :js, type: :system) do
     driven_by Capybara.javascript_driver
   end
 end

spec/support/javascript_errors.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 RSpec.configure do |config|
-  config.after(:each, type: :system) do
+  config.after(:each, :js, type: :system) do
     errors = page.driver.browser.logs.get(:browser)
     if errors.present?
       aggregate_failures 'javascript errrors' do

spec/support/streaming_server_manager.rb

@@ -95,7 +95,7 @@ RSpec.configure do |config|
     end
   end
 
-  config.around :each, type: :system do |example|
+  config.around :each, :streaming, type: :system do |example|
     # Streaming server needs DB access but `use_transactional_tests` rolls back
     # every transaction. Disable this feature for streaming tests, and use
     # DatabaseCleaner to clean the database tables between each test.
@@ -125,6 +125,6 @@ RSpec.configure do |config|
   end
 
   def streaming_examples_present?
-    RUN_SYSTEM_SPECS
+    RSpec.world.filtered_examples.values.flatten.any? { |example| example.metadata[:streaming] == true }
   end
 end

spec/system/new_statuses_spec.rb

@@ -2,7 +2,7 @@
 
 require 'rails_helper'
 
-describe 'NewStatuses', :sidekiq_inline do
+describe 'NewStatuses', :js, :sidekiq_inline, :streaming do
   include ProfileStories
 
   subject { page }

spec/system/oauth_spec.rb

@@ -2,8 +2,8 @@
 
 require 'rails_helper'
 
-describe 'Using OAuth from an external app' do
-  let(:client_app) { Doorkeeper::Application.create!(name: 'test', redirect_uri: 'http://localhost/health', scopes: 'read') }
+describe 'Using OAuth from an external app', :js, :streaming do
+  let(:client_app) { Doorkeeper::Application.create!(name: 'test', redirect_uri: about_url(host: Rails.application.config.x.local_domain), scopes: 'read') }
 
   context 'when the user is already logged in' do
     let!(:user) { Fabricate(:user) }

spec/system/ocr_spec.rb

@@ -2,7 +2,7 @@
 
 require 'rails_helper'
 
-describe 'OCR', :paperclip_processing, :sidekiq_inline do
+describe 'OCR', :js, :paperclip_processing, :sidekiq_inline, :streaming do
   include ProfileStories
 
   let(:email)               { 'test@example.com' }

spec/system/report_interface_spec.rb

@@ -2,7 +2,7 @@
 
 require 'rails_helper'
 
-describe 'report interface', :paperclip_processing do
+describe 'report interface', :js, :paperclip_processing, :streaming do
   include ProfileStories
 
   let(:email)               { 'admin@example.com' }

spec/system/share_entrypoint_spec.rb

@@ -2,7 +2,7 @@
 
 require 'rails_helper'
 
-describe 'ShareEntrypoint' do
+describe 'ShareEntrypoint', :js, :streaming do
   include ProfileStories
 
   subject { page }

spec/system/unlogged_spec.rb

@@ -2,7 +2,7 @@
 
 require 'rails_helper'
 
-describe 'UnloggedBrowsing' do
+describe 'UnloggedBrowsing', :js, :streaming do
   subject { page }
 
   before do

spec/workers/scheduler/user_cleanup_scheduler_spec.rb

@@ -14,7 +14,7 @@ describe Scheduler::UserCleanupScheduler do
     before do
       # Need to update the already-existing users because their initialization overrides confirmation_sent_at
       new_unconfirmed_user.update!(confirmed_at: nil, confirmation_sent_at: Time.now.utc)
-      old_unconfirmed_user.update!(confirmed_at: nil, confirmation_sent_at: 1.week.ago)
+      old_unconfirmed_user.update!(confirmed_at: nil, confirmation_sent_at: 10.days.ago)
       confirmed_user.update!(confirmed_at: 1.day.ago)
     end