From df752266dc5dd578597a3efb56f83a6eb878d29f Mon Sep 17 00:00:00 2001 From: aaronskiba <71047780+aaronskiba@users.noreply.github.com> Date: Tue, 30 Jan 2024 12:56:29 -0700 Subject: [PATCH 1/6] Use selenium-webdriver gem / Remove webdrivers According to https://github.com/titusfortner/webdrivers: "If you can update to the latest version of Selenium (4.11+), please do so and stop requiring this gem." --- Gemfile | 2 +- Gemfile.lock | 9 +++------ spec/rails_helper.rb | 4 +--- spec/spec_helper.rb | 11 ----------- spec/support/capybara.rb | 5 ----- 5 files changed, 5 insertions(+), 26 deletions(-) diff --git a/Gemfile b/Gemfile index 76fa7e71f3..cc16bb67a7 100644 --- a/Gemfile +++ b/Gemfile @@ -248,7 +248,7 @@ group :test do gem 'capybara' # Easy installation and use of web drivers to run system tests with browsers - gem 'webdrivers' + gem 'selenium-webdriver' # RSpec::CollectionMatchers lets you express expected outcomes on # collections of an object in an example. diff --git a/Gemfile.lock b/Gemfile.lock index 6635d71006..68c97fce60 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -460,7 +460,7 @@ GEM sawyer (0.9.2) addressable (>= 2.3.5) faraday (>= 0.17.3, < 3) - selenium-webdriver (4.10.0) + selenium-webdriver (4.16.0) rexml (~> 3.2, >= 3.2.5) rubyzip (>= 1.2.2, < 3.0) websocket (~> 1.0) @@ -517,10 +517,6 @@ GEM activemodel (>= 6.0.0) bindex (>= 0.4.0) railties (>= 6.0.0) - webdrivers (5.3.1) - nokogiri (~> 1.6) - rubyzip (>= 1.3.0) - selenium-webdriver (~> 4.0, < 4.11) webmock (3.19.1) addressable (>= 2.8.0) crack (>= 0.3.2) @@ -542,6 +538,7 @@ GEM PLATFORMS arm64-darwin-21 + arm64-darwin-22 x86_64-linux DEPENDENCIES @@ -605,6 +602,7 @@ DEPENDENCIES rubocop rubocop-i18n rubocop-performance + selenium-webdriver shoulda spring spring-commands-rspec @@ -613,7 +611,6 @@ DEPENDENCIES translation turbo-rails web-console - webdrivers webmock wicked_pdf wkhtmltopdf-binary diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 2872869549..c41e68fda4 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -39,11 +39,9 @@ # No need to run this during CI because we build the DB from the schema # ActiveRecord::Migration.maintain_test_schema! -# Block all external HTTP requests except to the Google APIs URL so that WebDrivers can fetch -# the latest Chromedrivers. +# Block all external HTTP requests WebMock.disable_net_connect!( allow_localhost: true, - allow: %w[chromedriver.storage.googleapis.com] ) # Configure RSpec diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index ddd7429517..c53a7e10ce 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -111,17 +111,6 @@ # Enable Capybara webmocks if we are testing a feature config.before(:each) do |example| - if example.metadata[:type] == :feature - # Capybara::Webmock.start - - # Allow Capybara to make localhost requests and also contact the - # google api chromedriver store - # WebMock.disable_net_connect!( - # allow_localhost: true, - # allow: %w[chromedriver.storage.googleapis.com] - # ) - end - # Ensure that there is always a default Language create(:language, abbreviation: 'en', default_language: true) unless Language.default.present? end diff --git a/spec/support/capybara.rb b/spec/support/capybara.rb index e8ac5b282d..ededee71dd 100644 --- a/spec/support/capybara.rb +++ b/spec/support/capybara.rb @@ -1,10 +1,5 @@ # frozen_string_literal: true -require 'webdrivers/chromedriver' - -# Cache for one hour -Webdrivers.cache_time = 3600 - # Use Puma as the webserver for feature tests Capybara.server = :puma, { Silent: true } From e0c62dac913ffad90c5e5bb7824d805734b8f8e1 Mon Sep 17 00:00:00 2001 From: aaronskiba <71047780+aaronskiba@users.noreply.github.com> Date: Tue, 30 Jan 2024 13:18:29 -0700 Subject: [PATCH 2/6] Don't limit sign-in attempts on test environment Without this modification, the selenium tests often trigger a 429 / Too Many Requests error. (Perhaps this should be applied to "/users/password" as well?) --- config/initializers/rack_attack.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb index 6a26816d6f..a9acbe74e8 100644 --- a/config/initializers/rack_attack.rb +++ b/config/initializers/rack_attack.rb @@ -21,5 +21,6 @@ # Throttle attempts to a particular path. 4 POSTs to /users/sign_in every 30 seconds Rack::Attack.throttle "logins/ip", limit: 4, period: 30.seconds do |req| - req.post? && req.path == "/users/sign_in" && req.ip + # Don't apply sign-in rate-limiting to test environment + req.post? && req.path == "/users/sign_in" && req.ip unless Rails.env.test? end From de5afd5ca4bcca6230e1a20998c161c0b2aad59a Mon Sep 17 00:00:00 2001 From: aaronskiba <71047780+aaronskiba@users.noreply.github.com> Date: Tue, 30 Jan 2024 13:25:51 -0700 Subject: [PATCH 3/6] replace 'safe_email' with 'email' for Faker Prior to this commit, the following deprecation warning was appearing in the terminal: NOTE: Faker::Internet.safe_email is deprecated; use email instead. It will be removed on or after 2023-10. More here: https://github.com/faker-ruby/faker/pull/2733 --- spec/factories/orgs.rb | 2 +- spec/factories/users.rb | 2 +- spec/models/plan_spec.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/factories/orgs.rb b/spec/factories/orgs.rb index a9f15f74b5..88bc858b0b 100644 --- a/spec/factories/orgs.rb +++ b/spec/factories/orgs.rb @@ -40,7 +40,7 @@ region { Region.first || create(:region) } language { Language.default } is_other { false } - contact_email { Faker::Internet.safe_email } + contact_email { Faker::Internet.email } contact_name { Faker::Name.name } managed { true } trait :institution do diff --git a/spec/factories/users.rb b/spec/factories/users.rb index 765898e944..c91c756604 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -57,7 +57,7 @@ language { Language.default } firstname { Faker::Name.unique.first_name } surname { Faker::Name.unique.last_name } - email { Faker::Internet.unique.safe_email } + email { Faker::Internet.unique.email } password { 'password' } accept_terms { true } diff --git a/spec/models/plan_spec.rb b/spec/models/plan_spec.rb index 2d9ef05a8e..08bcca6f5b 100644 --- a/spec/models/plan_spec.rb +++ b/spec/models/plan_spec.rb @@ -592,7 +592,7 @@ context 'when org contact_email present' do before do - org.update!(contact_email: Faker::Internet.safe_email) + org.update!(contact_email: Faker::Internet.email) end it 'emails the admins' do From d89c897f0c727ea4ef4f7464541383b97b8dd637 Mon Sep 17 00:00:00 2001 From: aaronskiba <71047780+aaronskiba@users.noreply.github.com> Date: Tue, 30 Jan 2024 13:26:34 -0700 Subject: [PATCH 4/6] Small tweaks to fix existing tests --- spec/features/annotations/annotations_editing_spec.rb | 2 +- spec/features/templates/templates_editings_spec.rb | 2 +- .../templates/templates_upgrade_customisations_spec.rb | 3 +-- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/spec/features/annotations/annotations_editing_spec.rb b/spec/features/annotations/annotations_editing_spec.rb index e3b5b8d68f..0edfc4a25e 100644 --- a/spec/features/annotations/annotations_editing_spec.rb +++ b/spec/features/annotations/annotations_editing_spec.rb @@ -56,7 +56,7 @@ expect { click_button 'Save' }.not_to change { Annotation.count } end expect(annotation.text).to eql('Foo bar') - expect(Annotation.order('created_at').last.text).to eql('Noo bar') + expect(Annotation.order('created_at').last.text).to eql('
Noo bar
') expect(page).not_to have_errors end diff --git a/spec/features/templates/templates_editings_spec.rb b/spec/features/templates/templates_editings_spec.rb index bc84511cd7..a56237f880 100644 --- a/spec/features/templates/templates_editings_spec.rb +++ b/spec/features/templates/templates_editings_spec.rb @@ -44,7 +44,7 @@ click_button 'Save' end # Make sure annotation has been updated - expect(Question.find(template.question_ids.first).annotations.first.text).to eql('Foo bar') + expect(Question.find(template.question_ids.first).annotations.first.text).to eql('Foo bar
') # Make sure blank records are not created for empty annotation form expect(Question.find(template.question_ids.first).annotations.count).to eql(1) expect(page).not_to have_errors diff --git a/spec/features/templates/templates_upgrade_customisations_spec.rb b/spec/features/templates/templates_upgrade_customisations_spec.rb index 4aa8eb9abc..9a5da195b0 100644 --- a/spec/features/templates/templates_upgrade_customisations_spec.rb +++ b/spec/features/templates/templates_upgrade_customisations_spec.rb @@ -76,8 +76,7 @@ end expect(page).to have_css('#new_question_new_question') within('#new_question_new_question') do - expect(find('#new_question_question_text')).to be_present - fill_in :new_question_question_text, with: 'Text for this specific question' + tinymce_fill_in :new_question_question_text, with: 'Text for this specific question' expect { click_button('Save') }.to change { Question.count }.by(1) end end From b1b38ecd83b4362a2f34bed8aa54f190047f54e4 Mon Sep 17 00:00:00 2001 From: aaronskiba <71047780+aaronskiba@users.noreply.github.com> Date: Wed, 31 Jan 2024 10:31:23 -0700 Subject: [PATCH 5/6] Remove duplicate `node-version:` from workflows Prior to this commit, the GitHub Actions associated with these files were failing with the message, "Invalid workflow file The workflow is not valid. .github/workflows/mysql.yml (Line: X, Col: Y): 'node-version' is already defined" --- .github/workflows/mysql.yml | 1 - .github/workflows/postgres.yml | 1 - 2 files changed, 2 deletions(-) diff --git a/.github/workflows/mysql.yml b/.github/workflows/mysql.yml index e561c8e8f6..69905aff98 100644 --- a/.github/workflows/mysql.yml +++ b/.github/workflows/mysql.yml @@ -27,7 +27,6 @@ jobs: with: node-version: '16.6.0' cache: 'yarn' - node-version: 16 # Copy all of the example configs over - name: 'Setup the application' diff --git a/.github/workflows/postgres.yml b/.github/workflows/postgres.yml index c9ce8115b2..12ba62b8e7 100644 --- a/.github/workflows/postgres.yml +++ b/.github/workflows/postgres.yml @@ -46,7 +46,6 @@ jobs: with: node-version: '16.6.0' cache: 'yarn' - node-version: 16 # Install the Postgres developer packages - name: 'Install Postgresql Packages' From 1d35aec7be275e3c7edddf1f20edb4c316b765ea Mon Sep 17 00:00:00 2001 From: aaronskiba <71047780+aaronskiba@users.noreply.github.com> Date: Wed, 7 Feb 2024 13:26:17 -0700 Subject: [PATCH 6/6] Make rubocop happy NOTE: Additional Rubocop offences currently exist in the codebase. These fixes only address the PR changes that correspond with this commit. --- config/initializers/rack_attack.rb | 2 +- spec/rails_helper.rb | 2 +- spec/spec_helper.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb index a9acbe74e8..c8ca0f574c 100644 --- a/config/initializers/rack_attack.rb +++ b/config/initializers/rack_attack.rb @@ -21,6 +21,6 @@ # Throttle attempts to a particular path. 4 POSTs to /users/sign_in every 30 seconds Rack::Attack.throttle "logins/ip", limit: 4, period: 30.seconds do |req| - # Don't apply sign-in rate-limiting to test environment + # Don't apply sign-in rate-limiting to test environment req.post? && req.path == "/users/sign_in" && req.ip unless Rails.env.test? end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index c41e68fda4..98c265b5cf 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -41,7 +41,7 @@ # Block all external HTTP requests WebMock.disable_net_connect!( - allow_localhost: true, + allow_localhost: true ) # Configure RSpec diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index c53a7e10ce..2904a6894d 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -110,7 +110,7 @@ end # Enable Capybara webmocks if we are testing a feature - config.before(:each) do |example| + config.before(:each) do # Ensure that there is always a default Language create(:language, abbreviation: 'en', default_language: true) unless Language.default.present? end