From 10d0880dd282552fdf277463c61d63a386e0bafc Mon Sep 17 00:00:00 2001
From: adi-herwana-nus <adi.hwn@nus.edu.sg>
Date: Mon, 23 Sep 2024 15:38:31 +0800
Subject: [PATCH] test: improved rspec test reliability

- updated Keycloak config to allow user delete/overwrite from DB
- refactored various tests/helper functions to phase out sleep stmts
- added sleep statements to account for page animations
- added search queries to tests looking for objects in paginated lists
- increased Capybara max wait time from 5s to 10s
---
 .circleci/config.yml                          |  2 +-
 authentication/import/coursemology_realm.json |  8 +--
 .../question/scribing_controller_spec.rb      |  3 +-
 spec/factories/courses.rb                     |  7 ++-
 spec/factories/generic_announcements.rb       |  5 +-
 spec/factories/instance_users.rb              | 11 +++-
 spec/features/course/admin/admin_spec.rb      |  3 +-
 .../assessment/assessment_viewing_spec.rb     |  9 ++--
 .../question/programming_management_spec.rb   | 31 +++++++----
 .../question/text_response_management_spec.rb | 26 ++++------
 .../admin/announcement_management_spec.rb     | 42 +++++++++------
 .../system/admin/components_settings_spec.rb  |  7 +--
 .../admin/instance/course_management_spec.rb  | 52 ++++++++++++-------
 .../admin/instance/user_management_spec.rb    | 45 ++++++++++------
 spec/spec_helper.rb                           |  2 +-
 spec/support/active_job.rb                    | 11 ++++
 spec/support/authentication_performers.rb     |  9 ++--
 spec/support/capybara.rb                      |  4 +-
 18 files changed, 167 insertions(+), 110 deletions(-)

diff --git a/.circleci/config.yml b/.circleci/config.yml
index 764553aa30e..05fa0019bf4 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -221,7 +221,7 @@ commands:
     steps:
       - run:
           name: Terminate Rails server
-          command: pkill -SIGTERM -f puma
+          command: pkill -SIGINT -f puma
 
       - run:
           name: Wait for Rails coverage results
diff --git a/authentication/import/coursemology_realm.json b/authentication/import/coursemology_realm.json
index 2fc36e7bd7c..708c8ac2357 100644
--- a/authentication/import/coursemology_realm.json
+++ b/authentication/import/coursemology_realm.json
@@ -1797,7 +1797,7 @@
             "true"
           ],
           "allowKeycloakDelete": [
-            "false"
+            "true"
           ],
           "findBySearchTerm": [
             "select ue.id as \"id\", users.id as \"user_id\", ue.email as \"username\", ue.email as \"email\", users.name as \"firstName\", CASE WHEN ue.confirmed_at IS NOT NULL THEN 'TRUE' ELSE NULL END as \"EMAIL_VERIFIED\"from user_emails ue left join users on ue.user_id = users.id where LOWER(ue.email) like LOWER(concat('%', ?, '%')) or LOWER(users.name) like LOWER(concat('%', ?, '%'))"
@@ -1821,7 +1821,7 @@
             "postgres"
           ],
           "allowDatabaseToOverwriteKeycloak": [
-            "false"
+            "true"
           ]
         }
       }
@@ -4189,7 +4189,7 @@
             "true"
           ],
           "allowKeycloakDelete": [
-            "false"
+            "true"
           ],
           "findBySearchTerm": [
             "select ue.id as \"id\", users.id as \"user_id\", ue.email as \"username\", ue.email as \"email\", users.name as \"firstName\", CASE WHEN ue.confirmed_at IS NOT NULL THEN 'TRUE' ELSE NULL END as \"EMAIL_VERIFIED\"from user_emails ue left join users on ue.user_id = users.id where LOWER(ue.email) like LOWER(concat('%', ?, '%')) or LOWER(users.name) like LOWER(concat('%', ?, '%'))"
@@ -4210,7 +4210,7 @@
             "postgres"
           ],
           "allowDatabaseToOverwriteKeycloak": [
-            "false"
+            "true"
           ]
         }
       }
diff --git a/spec/controllers/course/assessment/question/scribing_controller_spec.rb b/spec/controllers/course/assessment/question/scribing_controller_spec.rb
index 29515f93064..02d95ea1faf 100644
--- a/spec/controllers/course/assessment/question/scribing_controller_spec.rb
+++ b/spec/controllers/course/assessment/question/scribing_controller_spec.rb
@@ -81,10 +81,10 @@
             result[:file] = fixture_file_upload(file_path, 'application/pdf')
           end
         end
+        let(:file_path) { 'files/one-page-document.pdf' }
 
         # "CircleCI's imagemagick installation is flaky"
         pending 'when the pdf is one page' do
-          let(:file_path) { 'files/one-page-document.pdf' }
           it 'creates one scribing question with a png attachment' do
             expect { subject }.to change { Course::Assessment::Question::Scribing.count }.by(1)
 
@@ -99,7 +99,6 @@
 
         # "CircleCI's imagemagick installation is flaky"
         pending 'when the pdf is two pages' do
-          let(:file_path) { 'files/two-page-document.pdf' }
           it 'creates one scribing question with a png attachment for each pdf page' do
             expect { subject }.to change { Course::Assessment::Question::Scribing.count }.by(2)
 
diff --git a/spec/factories/courses.rb b/spec/factories/courses.rb
index 7159853e1d2..e3595d85f2c 100644
--- a/spec/factories/courses.rb
+++ b/spec/factories/courses.rb
@@ -1,9 +1,12 @@
 # frozen_string_literal: true
 FactoryBot.define do
   factory :course do
+    transient do
+      prefix { 'Course ' }
+    end
     sequence(:title) do |n|
-      timestamp = Time.zone.now.to_i.to_s
-      "Course #{timestamp + n.to_s}"
+      timestamp = Time.zone.now.to_i.to_s(36)
+      "#{prefix}#{timestamp}#{n}"
     end
     description { 'example course' }
     start_at { Time.zone.now }
diff --git a/spec/factories/generic_announcements.rb b/spec/factories/generic_announcements.rb
index ca9bdc9a50e..5ae1e3ff10d 100644
--- a/spec/factories/generic_announcements.rb
+++ b/spec/factories/generic_announcements.rb
@@ -1,7 +1,10 @@
 # frozen_string_literal: true
 FactoryBot.define do
   factory :generic_announcement, class: System::Announcement.name do
-    sequence(:title) { |n| "Announcement #{n}" }
+    transient do
+      prefix { 'Announcement ' }
+    end
+    sequence(:title) { |n| "#{prefix}#{n}" }
     sequence(:content) { |n| "Content #{n}" }
 
     start_at { Time.zone.now }
diff --git a/spec/factories/instance_users.rb b/spec/factories/instance_users.rb
index a16f0da0673..1c27862de5a 100644
--- a/spec/factories/instance_users.rb
+++ b/spec/factories/instance_users.rb
@@ -1,11 +1,18 @@
 # frozen_string_literal: true
 FactoryBot.define do
   factory :instance_user do
+    transient do
+      user_name { nil }
+    end
     instance
     role { :normal }
 
-    after(:build) do |instance_user|
-      instance_user.user ||= build(:user, instance_users: [instance_user])
+    after(:build) do |instance_user, evaluator|
+      instance_user.user ||= if evaluator.user_name
+                               build(:user, name: evaluator.user_name, instance_users: [instance_user])
+                             else
+                               instance_user.user ||= build(:user, instance_users: [instance_user])
+                             end
     end
 
     trait :instructor do
diff --git a/spec/features/course/admin/admin_spec.rb b/spec/features/course/admin/admin_spec.rb
index fa52ddf2e16..72da7e2525d 100644
--- a/spec/features/course/admin/admin_spec.rb
+++ b/spec/features/course/admin/admin_spec.rb
@@ -47,9 +47,8 @@
           find('label', text: 'Change', visible: false).click
         end
 
-        click_button 'Done'
+        find("[role='dialog']").find('button', text: 'Done').click
         click_button 'Save changes'
-        wait_for_page
         expect_toastify('The new course logo was successfully uploaded.')
 
         visit current_path
diff --git a/spec/features/course/assessment/assessment_viewing_spec.rb b/spec/features/course/assessment/assessment_viewing_spec.rb
index 5f43af4489e..a4d103c72b3 100644
--- a/spec/features/course/assessment/assessment_viewing_spec.rb
+++ b/spec/features/course/assessment/assessment_viewing_spec.rb
@@ -14,17 +14,14 @@
 
       scenario 'I can access all submissions of an assessment' do
         assessment
-        visit course_assessments_path(course)
-        wait_for_page
-
-        submissions_button = find_link('Submissions', visible: false)
-        hover_then_click submissions_button
 
+        visit course_assessments_path(course)
+        hover_then_click find('a[aria-label="Submissions"]')
         expect(current_path).to eq(course_assessment_submissions_path(course, assessment))
 
         # Access submissions from the show assessment page
         visit course_assessment_path(course, assessment)
-        click_link 'Submissions'
+        hover_then_click find('a[aria-label="Submissions"]')
         expect(current_path).to eq(course_assessment_submissions_path(course, assessment))
       end
 
diff --git a/spec/features/course/assessment/question/programming_management_spec.rb b/spec/features/course/assessment/question/programming_management_spec.rb
index fb02c5d38c5..600404cd397 100644
--- a/spec/features/course/assessment/question/programming_management_spec.rb
+++ b/spec/features/course/assessment/question/programming_management_spec.rb
@@ -16,6 +16,7 @@
         skill = create(:course_assessment_skill, course: course)
         visit course_assessment_path(course, assessment)
         click_on 'New Question'
+        wait_for_animation
         new_page = window_opened_by { click_link 'Programming' }
 
         within_window new_page do
@@ -34,17 +35,19 @@
           find('li', text: skill.title).click
 
           find_all('div', text: 'Language').last.click
-          find('li', text: attributes[:language].name).click
+          wait_for_animation
+          find('li', exact_text: attributes[:language].name).click
 
           find('div', id: 'testUi.metadata.submission').click
           send_keys template
 
           click_button 'Save changes'
-          wait_for_page
+          expect_toastify('Question saved.')
 
           expect(page).to have_current_path(course_assessment_path(course, assessment))
 
           new_question = assessment.questions.first.specific.reload
+          expect(assessment.questions.count).to eq(1)
           expect(new_question.title).to eq(attributes[:title])
           expect(new_question.maximum_grade).to eq(attributes[:maximum_grade])
           expect(new_question.description).to include(attributes[:description])
@@ -77,7 +80,8 @@
         expect(page).not_to have_field('Attempt limit')
 
         find_all('div', text: 'Language').last.click
-        find('li', text: attributes[:language].name).click
+        wait_for_animation
+        find('li', exact_text: attributes[:language].name).click
 
         find('span', text: 'Evaluate and test code').click
         expect(page).to have_text('submissions will always receive the maximum grade')
@@ -101,11 +105,12 @@
         end
 
         click_button 'Save changes'
-        wait_for_page
+        expect_toastify('Question saved.')
 
         expect(page).to have_current_path(course_assessment_path(course, assessment))
 
         new_question = assessment.questions.first.specific.reload
+        expect(assessment.questions.count).to eq(1)
         expect(new_question.title).to eq(attributes[:title])
         expect(new_question.maximum_grade).to eq(attributes[:maximum_grade])
         expect(new_question.description).to include(attributes[:description])
@@ -130,17 +135,20 @@
         end
 
         click_button 'Save changes'
-        wait_for_page
-
+        expect_toastify("package wasn't successfully imported")
         expect(page).to have_text('error')
-        expect_toastify "package wasn't successfully imported"
+        find('.Toastify').find('svg[data-testid="CloseIcon"]').click
 
         attach_file(valid_package) do
           click_button 'Upload a new package'
         end
 
+        # TODO: close button on toastify messages doesn't work,
+        # but interacting with the page causes it to close eventually
+        expect(page).to_not have_css('.Toastify', text: "package wasn't successfully imported", wait: 60)
+
         click_button 'Save changes'
-        wait_for_page
+        expect_toastify('Question saved.')
 
         expect(page).to have_current_path(course_assessment_path(course, assessment))
 
@@ -169,7 +177,7 @@
 
         fill_in 'Maximum grade', with: maximum_grade
         click_button 'Save changes'
-        wait_for_page
+        expect_toastify('Question saved.')
 
         expect(page).to have_current_path(course_assessment_path(course, assessment))
         expect(question.reload.maximum_grade).to eq(maximum_grade)
@@ -193,6 +201,7 @@
         fill_in 'Maximum grade', with: attributes[:maximum_grade]
 
         find_all('div', text: 'Language').last.click
+        wait_for_animation
         find('li', text: attributes[:language].name).click
 
         find('span', text: 'Evaluate and test code').click
@@ -208,9 +217,9 @@
         end
 
         click_button 'Save changes'
-        wait_for_page
+        expect_toastify('Question saved.')
 
-        new_question = assessment.questions.first.specific
+        new_question = assessment.questions.first.specific.reload
         expect(new_question.memory_limit).to eq(attributes[:memory_limit])
         expect(new_question.time_limit).to eq(attributes[:time_limit])
         expect(new_question.attempt_limit).to eq(attributes[:attempt_limit])
diff --git a/spec/features/course/assessment/question/text_response_management_spec.rb b/spec/features/course/assessment/question/text_response_management_spec.rb
index 122b7790c2e..0ecadbbc882 100644
--- a/spec/features/course/assessment/question/text_response_management_spec.rb
+++ b/spec/features/course/assessment/question/text_response_management_spec.rb
@@ -19,11 +19,12 @@
         skill = create(:course_assessment_skill, course: course)
         visit course_assessment_path(course, assessment)
         click_on 'New Question'
+        wait_for_animation
         new_page = window_opened_by { click_link 'File Upload' }
 
         within_window new_page do
-          expect(current_path).to eq(
-            new_course_assessment_question_text_response_path(course, assessment)
+          expect(page).to have_current_path(
+            new_course_assessment_question_text_response_path(course, assessment, { file_upload: true })
           )
           question_attributes = attributes_for(:course_assessment_question_text_response)
           fill_in 'title', with: question_attributes[:title]
@@ -36,7 +37,7 @@
           find('li', text: skill.title).click
 
           click_button 'Save changes'
-          wait_for_page
+          expect(page).to have_current_path(course_assessment_path(course, assessment))
 
           question_created = assessment.questions.first.specific
           expect(question_created.question_assessments.first.skills).to contain_exactly(skill)
@@ -52,11 +53,11 @@
       scenario 'I can create a new text response question' do
         visit course_assessment_path(course, assessment)
         click_on 'New Question'
-        new_page = window_opened_by { click_link 'Text Response' }
-
+        wait_for_animation
+        new_page = window_opened_by { click_link(exact_text: 'Text Response') }
         within_window new_page do
           file_upload_path = new_course_assessment_question_text_response_path(course, assessment)
-          expect(current_path).to eq(file_upload_path)
+          expect(page).to have_current_path(file_upload_path)
 
           question_attributes = attributes_for(:course_assessment_question_text_response)
           fill_in 'title', with: question_attributes[:title]
@@ -65,7 +66,7 @@
           fill_in 'maximumGrade', with: question_attributes[:maximum_grade]
 
           click_button 'Save changes'
-          wait_for_page
+          expect(page).to have_current_path(course_assessment_path(course, assessment))
 
           question_created = assessment.questions.first.specific
           expect(question_created.title).to eq(question_attributes[:title])
@@ -100,9 +101,8 @@
         fill_in 'maximumGrade', with: maximum_grade
 
         click_button 'Save changes'
-        wait_for_page
 
-        expect(current_path).to eq(course_assessment_path(course, assessment))
+        expect(page).to have_current_path(course_assessment_path(course, assessment))
         expect(question.reload.title).to eq(title)
         expect(question.reload.description).to include(description)
         expect(question.reload.staff_only_comments).to include(staff_only_comments)
@@ -129,9 +129,8 @@
         end
 
         click_button 'Save changes'
-        wait_for_page
 
-        expect(current_path).to eq(course_assessment_path(course, assessment))
+        expect(page).to have_current_path(course_assessment_path(course, assessment))
         expect(question.reload.max_attachments).to eq(max_attachment_default_text_response)
         expect(question.reload.solutions.count).to eq(solutions.count)
 
@@ -140,17 +139,14 @@
 
         find_all('button[aria-label="Delete solution"]').each(&:click)
         click_button 'Save changes'
-        wait_for_page
 
-        expect(current_path).to eq(course_assessment_path(course, assessment))
+        expect(page).to have_current_path(course_assessment_path(course, assessment))
         expect(question.reload.solutions.count).to eq(0)
       end
 
       scenario 'I can delete a text response question' do
-        skip 'Flaky tests'
         question = create(:course_assessment_question_text_response, assessment: assessment)
         visit course_assessment_path(course, assessment)
-        wait_for_page
 
         within find('section', text: question.title) { click_button 'Delete' }
         click_button 'Delete question'
diff --git a/spec/features/system/admin/announcement_management_spec.rb b/spec/features/system/admin/announcement_management_spec.rb
index 15bcae1a7b1..e40007af61f 100644
--- a/spec/features/system/admin/announcement_management_spec.rb
+++ b/spec/features/system/admin/announcement_management_spec.rb
@@ -4,16 +4,28 @@
 RSpec.feature 'System: Administration: Announcements' do
   let(:instance) { Instance.default }
   with_tenant(:instance) do
+    let!(:prefix) { "testadm-#{rand(36**12).to_s(36)}-ann-" }
     before do
       login_as(user, scope: :user)
     end
 
+    # For certain tests, use the search box to only show the announcements we created for this test.
+    # This is to prevent tests failing if there are so many announcements such that
+    # the ones created for this test aren't on the first page.
+    def search_for_announcements(query)
+      find(
+        :xpath,
+        # Find the announcements feed, go up to the parent, and select the input text field under it
+        # (the Search text field)
+        '//div[@id="course-announcements"]/parent::*/descendant::input[@type="text"]'
+      ).set('').native.send_keys(query)
+    end
+
     context 'As a system administrator', js: true do
       let(:user) { create(:administrator) }
 
       scenario 'I can create announcements' do
         visit admin_announcements_path
-        wait_for_page
 
         find('button#new-announcement-button').click
 
@@ -22,18 +34,20 @@
         fill_in_react_ck 'textarea[name="content"]', announcement[:content]
 
         find('button.btn-submit').click
-        wait_for_page
+
+        expect_toastify('New announcement posted!')
         expect(current_path).to eq(admin_announcements_path)
+
+        search_for_announcements(announcement[:title])
         expect(page).to have_text(announcement[:title])
         expect(page).to have_text(announcement[:content])
-        expect_toastify('New announcement posted!')
       end
 
       scenario 'I can edit announcements' do
-        announcement = create(:system_announcement)
+        announcement = create(:system_announcement, prefix: prefix)
         visit admin_announcements_path
-        wait_for_page
 
+        search_for_announcements(announcement.title)
         find("#announcement-edit-button-#{announcement.id}").click
 
         fill_in 'title', with: 'long string' * 100
@@ -41,13 +55,12 @@
         expect_toastify('Failed to update the announcement')
         expect(page).to have_selector('#form-dialog-update-button')
 
-        new_title = 'New Title'
+        new_title = announcement.title.upcase
         fill_in 'title', with: new_title
         find('#form-dialog-update-button').click
 
-        # Commented due to flaky test
-        # expect_toastify('Announcement updated')
-        wait_for_page
+        expect_toastify('Announcement updated')
+        search_for_announcements(new_title)
 
         expect(current_path).to eq admin_announcements_path
         within find("#announcement-#{announcement.id}") do
@@ -57,12 +70,12 @@
       end
 
       scenario 'I can see all announcements' do
-        create_list(:system_announcement, 2)
-        announcements = System::Announcement.first(2)
+        search_prefix = "testadm-search-#{rand(36**12).to_s(36)}-ann-"
+        announcements = create_list(:system_announcement, 2, prefix: search_prefix)
         visit admin_announcements_path
         expect(page).to have_selector('#new-announcement-button')
-        wait_for_page
 
+        search_for_announcements(search_prefix)
         announcements.each do |announcement|
           expect(page).to have_selector("#announcement-#{announcement.id}")
           expect(page).to have_selector("#announcement-edit-button-#{announcement.id}")
@@ -71,10 +84,9 @@
       end
 
       scenario 'I can delete announcements' do
-        create(:system_announcement)
-        announcement = System::Announcement.first
+        announcement = create(:system_announcement, prefix: prefix)
         visit admin_announcements_path
-        wait_for_page
+        search_for_announcements(announcement.title)
 
         find("#announcement-delete-button-#{announcement.id}").click
         click_button('Delete')
diff --git a/spec/features/system/admin/components_settings_spec.rb b/spec/features/system/admin/components_settings_spec.rb
index f0fc0a5cb75..7d88f5d33b3 100644
--- a/spec/features/system/admin/components_settings_spec.rb
+++ b/spec/features/system/admin/components_settings_spec.rb
@@ -12,7 +12,7 @@
       login_as(admin, scope: :user)
     end
 
-    scenario 'Admin visits the page' do
+    scenario 'Admin visits the page and enable/disable component' do
       visit admin_instance_components_path
       settings = Instance::Settings::Components.new(instance)
       enabled_components = settings.enabled_component_ids
@@ -29,12 +29,7 @@
           end
         end
       end
-    end
 
-    scenario 'Enable/disable a component' do
-      visit admin_instance_components_path
-      settings = Instance::Settings::Components.new(instance)
-      enabled_components = settings.enabled_component_ids
       component_to_modify = enabled_components.sample
 
       within find("tr#component_#{component_to_modify}") do
diff --git a/spec/features/system/admin/instance/course_management_spec.rb b/spec/features/system/admin/instance/course_management_spec.rb
index c428eda43fd..e8b12cb9a3d 100644
--- a/spec/features/system/admin/instance/course_management_spec.rb
+++ b/spec/features/system/admin/instance/course_management_spec.rb
@@ -6,33 +6,48 @@
 
   with_tenant(:instance) do
     let(:last_page) { Course.unscoped.page.total_pages }
+    let!(:prefix) { "testadm-#{rand(36**12).to_s(36)}-crs-" }
     let!(:courses) do
-      courses = create_list(:course, 2)
+      courses = create_list(:course, 2, prefix: prefix)
       create(:course_manager, course: courses.sample)
       create(:course_student, course: courses.sample)
 
       courses
     end
 
+    # For certain tests, use the search box to only show the courses we created for this test.
+    # This is to prevent tests failing if there are so many courses such that
+    # the ones created for this test aren't on the first page.
+    def search_for_courses(query)
+      find(
+        :xpath,
+        # Find the courses table, go up to the parent, and select the input text field under that parent
+        # (the Search text field)
+        '//div[contains(@class, "MuiTableContainer-root")]/parent::*/descendant::input[@type="text"]'
+      ).native.send_keys(query)
+    end
+
     context 'As a Instance Administrator' do
       let(:admin) { create(:instance_administrator).user }
       before { login_as(admin, scope: :user) }
 
       scenario 'I can view all courses in the instance' do
         visit admin_instance_courses_path
+        search_for_courses(prefix)
 
         courses.each do |course|
-          expect(page).to have_selector("tr.course_#{course.id}", text: course.title)
-          expect(page).
-            to have_link(nil, href: "//#{course.instance.host}/courses/#{course.id}")
+          within find("tr.course_#{course.id}", text: course.title) do
+            expect(page).
+              to have_link(nil, href: "//#{course.instance.host}/courses/#{course.id}")
 
-          # It shows and only shows the owners
-          course.course_users.owner.each do |course_user|
-            expect(page).to have_selector('li', text: course_user.user.name)
-          end
+            # It shows and only shows the owners
+            course.course_users.owner.each do |course_user|
+              expect(page).to have_selector('li', exact_text: course_user.user.name)
+            end
 
-          course.course_users.reject(&:owner?).each do |course_user|
-            expect(page).not_to have_selector('li', text: course_user.user.name)
+            course.course_users.reject(&:owner?).each do |course_user|
+              expect(page).not_to have_selector('li', exact_text: course_user.user.name)
+            end
           end
         end
       end
@@ -40,27 +55,24 @@
       let!(:course_to_delete) { create(:course) }
       scenario 'I can delete a course' do
         visit admin_instance_courses_path
+        search_for_courses(course_to_delete.title)
 
         find("button.course-delete-#{course_to_delete.id}").click
         expect(page).to have_button('Delete course', disabled: true)
         fill_in 'confirmDeleteField', with: 'coursemology'
         click_button('Delete course')
-        wait_for_page
         expect_toastify("#{course_to_delete.title} was deleted.")
       end
 
-      let!(:course_to_search) { create(:course) }
+      let!(:course_to_search) { create(:course, prefix: "testadm-search-#{rand(36**12).to_s(36)}-crs-") }
       scenario 'I can search courses' do
-        skip 'Flaky tests'
         visit admin_instance_courses_path
+        search_for_courses(course_to_search.title)
 
-        find_button('Search').click
-        find('div[aria-label="Search"]').find('input').set(course_to_search.title)
-
-        wait_for_field_debouncing # timeout for search debouncing
-
-        expect(page).to have_selector('p.course_title', text: course_to_search.title)
-        expect(all('.course').count).to eq(1)
+        within find('div.MuiTableContainer-root') do
+          expect(page).to have_text(course_to_search.title)
+          expect(page.first('tbody')).to have_selector('tr', count: 1)
+        end
       end
     end
   end
diff --git a/spec/features/system/admin/instance/user_management_spec.rb b/spec/features/system/admin/instance/user_management_spec.rb
index 9ab72d2915e..953cd039104 100644
--- a/spec/features/system/admin/instance/user_management_spec.rb
+++ b/spec/features/system/admin/instance/user_management_spec.rb
@@ -6,7 +6,22 @@
 
   with_tenant(:instance) do
     let(:instance_admin) { create(:instance_user, role: :administrator).user }
-    let!(:instance_users) { create_list(:instance_user, 2) }
+    let!(:prefix) { "testadm-#{rand(36**12).to_s(36)}-usr-" }
+    let!(:instance_users) do
+      (1..2).map do |i|
+        create(:instance_user, user_name: "#{prefix}#{i}")
+      end
+    end
+
+    # For certain tests, use the search box to only show the users we created for this test.
+    # This is to prevent tests failing if there are so many users such that
+    # the ones created for this test aren't on the first page.
+    def search_for_users(query, click: true)
+      within find('div[aria-label="Table Toolbar"]') do
+        find('button[aria-label="Search"]').click if click
+        find('input[type="text"]').set('').native.send_keys(query)
+      end
+    end
 
     context 'As a Instance Administrator' do
       before { login_as(instance_admin, scope: :user) }
@@ -14,6 +29,7 @@
       scenario 'I can view all users in the instance' do
         visit admin_instance_users_path
 
+        search_for_users(prefix)
         instance_users.each do |instance_user|
           expect(page).to have_text(instance_user.user.name)
           expect(page).to have_selector('p.user_email', text: instance_user.user.email)
@@ -36,11 +52,11 @@
         expect(page).to have_selector('p.user_email', exact_text: instance_admin.email)
       end
 
-      # Flaky test
-      xscenario "I can change a user's role" do
+      scenario "I can change a user's role" do
         visit admin_instance_users_path
 
         user_to_change = instance_users.sample
+        search_for_users(user_to_change.user.name)
 
         within find("tr.instance_user_#{user_to_change.id}") do
           find('div.user_role').click
@@ -54,37 +70,36 @@
       scenario 'I can delete a user' do
         visit admin_instance_users_path
 
+        search_for_users(prefix)
         user_to_delete = instance_users.sample
         find("button.user-delete-#{user_to_delete.id}").click
         accept_prompt
         expect_toastify('User was deleted.')
       end
 
+      # Generate new users to search so it doesn't conflict with above scenarios
       scenario 'I can search users' do
-        skip 'Flaky tests'
-        user_name = 'lool'
-        instance_users_to_search = create_list(:user, 2, name: user_name).
-                                   map { |u| u.instance_users.first }
+        search_prefix = "testadm-search-#{rand(36**12).to_s(36)}-usr-"
+        instance_users_to_search = (1..2).map do |i|
+          create(:instance_user, user_name: "#{search_prefix}#{i}")
+        end
+
         visit admin_instance_users_path
 
         # Search by username
-        click_button 'Search'
-        find('div[aria-label="Search"]').find('input').set(user_name)
-
-        wait_for_field_debouncing # timeout for search debouncing
+        search_for_users(search_prefix)
 
         instance_users_to_search.each do |instance_user|
           expect(page).to have_text(instance_user.user.name)
         end
-        expect(all('.instance_user').count).to eq(2)
+        expect(page).to have_selector('.instance_user', count: 2)
 
         # Search by email
         random_instance_user = InstanceUser.order('RANDOM()').first
-        find('div[aria-label="Search"]').find('input').set(random_instance_user.user.email)
-        wait_for_field_debouncing # timeout for search debouncing
+        search_for_users(random_instance_user.user.email, click: false)
 
         expect(page).to have_text(random_instance_user.user.name)
-        expect(all('.instance_user').count).to eq(1)
+        expect(page).to have_selector('.instance_user', count: 1)
       end
     end
   end
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index bdf79b3145d..75b917a17bb 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -122,7 +122,7 @@
 
 Capybara.configure do |config|
   config.server = :puma, { Silent: true }
-  config.default_max_wait_time = 5
+  config.default_max_wait_time = 10
   config.enable_aria_label = true
   config.server_host = Application::Application.config.x.default_host
   config.server_port = Application::Application.config.x.server_port
diff --git a/spec/support/active_job.rb b/spec/support/active_job.rb
index af329c694bf..e96cf0bc0fc 100644
--- a/spec/support/active_job.rb
+++ b/spec/support/active_job.rb
@@ -62,6 +62,10 @@ def wait_for_job
     end
   end
 
+  # TODO: Whenever possible, rewrite tests currently using these functions to use alternative solutions.
+  # Sleeping for a preset time is inherently flaky as the page may not always behave the same way.
+  # https://thoughtbot.com/blog/write-reliable-asynchronous-integration-tests-with-capybara
+
   # Wait for page/react lifecycle to finish loading/end
   def wait_for_page
     sleep 2.5
@@ -76,6 +80,13 @@ def wait_for_field_debouncing
     sleep 0.5
   end
 
+  # Wait for certain animations to finish before performing further actions (e.g. clicking a list element)
+  # The animation may cause tests to click the wrong element if it happens at just the right time.
+  # As with all the wait_for_* methods, we should stop using this when a better solution is found.
+  def wait_for_animation
+    sleep 1
+  end
+
   def visit_current_path
     tries ||= 10
     visit current_path
diff --git a/spec/support/authentication_performers.rb b/spec/support/authentication_performers.rb
index d2835a3a1a4..d6abfba83bd 100644
--- a/spec/support/authentication_performers.rb
+++ b/spec/support/authentication_performers.rb
@@ -16,16 +16,15 @@ def login_as(user, _ = {})
     fill_in 'Password', with: password_for(user)
     click_button 'Sign In'
 
-    wait_for_page
+    # We expect all authenticated pages should have a user menu button.
+    expect(page).to have_css('div[data-testid="user-menu-button"]')
   end
 
   def logout(*_)
-    # We expect all pages should have a user menu button.
     find('div[data-testid="user-menu-button"]').click
     find('li', text: 'Sign out').click
-    click_button 'Logout'
-
-    wait_for_page
+    click_button('Logout')
+    expect(page).to_not have_css('div[data-testid="user-menu-button"]')
   end
 
   private
diff --git a/spec/support/capybara.rb b/spec/support/capybara.rb
index 35a5598ece8..23bb1818eb1 100644
--- a/spec/support/capybara.rb
+++ b/spec/support/capybara.rb
@@ -87,10 +87,10 @@ def fill_in_mui_datetime(field_name, date)
     # Since capybara's `find` has a default timeout until the element is found, this helps
     # to ensure certain changes are made before continuing with the tests.
     def expect_toastify(message)
-      wait_for_page # To ensure toast is open
+      expect(page).to have_css('.Toastify', visible: true)
       container = find_all('.Toastify').first
       expect(container).to have_text(message)
-      find('p', text: message).click
+      container.find('p', text: message).click
     end
 
     # Finds a react-beautiful-dnd draggable element