Race Condition Check: spec/system/banners/

[FireLemons]

Overview

Our system tests run on a browser. Sometimes Github's servers are strained so the browser performs very slowly. For some tests this causes a race condition between entering data using a the browser and checking the effect of the entered data through ActiveRecord which bypasses the browser. The database check happens before the browser actions are completed if the browser's performance is crippled enough.

Examples:

Good expects

  expect(page).to have_content "Court Date"
  expect(page).to have_text(supervisor_name)
  
  wait_for_download
  expect(download_docx.paragraphs.map(&:to_s)).to include("Hearing Date: January 8, 2021")

Bad/Suspicious expects

  expect(CourtDate.count).to eq 2
  expect(supervisor.reload).not_to be_active
  
  deliveries = ActionMailer::Base.deliveries
  expect(deliveries.count).to eq(1)
  expect(deliveries.last.subject).to have_text "CASA Console invitation instructions"

The bad expects don't get their data from the browser. If they don't get the data from the browser, it doesn't make them automatically invalid. If the test waits for the page to load before checking data from the backend, that's valid. However for a system test, we should try to write them so all the steps of the test would have meaning to a user. A user would be able to see elements on a webpage but they wouldn't be able to check the database.

Things to do

• Check these files:

  • spec/system/banners/dismiss_spec.rb
  • spec/system/banners/new_spec.rb

• Check all expects and make sure each one checks something on the webpage.

• If an expect checks anything other than the webpage or resources accessible from the webpage(like downloads), rewrite the expect to check the webpage.

• If there are no expects to replace, comment with some proof.

[vishaltps]

I'd like to work on this issue. Please assign it to me.

[vishaltps]

@FireLemons I reviewed both files and all expects already check the webpage. No changes needed.

spec/system/banners/dismiss_spec.rb

• 2 expects, both check the page:
  • expect(page).to have_text("Please fill out this survey") (line 13)
  • expect(page).not_to have_text("Please fill out this survey") (line 16)

spec/system/banners/new_spec.rb

• 14 expects, all check the page:
  • 10x expect(page).to have_text(...)
  • 1x expect(page).not_to have_text(...)
  • 1x expect(page).to have_current_path(...)
  • 2x expect(..._row).to have_selector(...) (Capybara element — still webpage)

No instances of non-webpage assertions (.count, .reload, ActionMailer::Base.deliveries, Model.find_by, etc.) were found in either file.