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

@@ -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

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"
           ]
         }
       }

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)
 

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 }

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 }

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

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

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
 

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])

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'