There were many cases where we were clicking on a link or (most of the
time) a button and then calling the `visit` method. In the past, it
worked just fine because clicking on buttons usually results in non-AJAX
requests, meaning that the test waited for the request to finish before
continuing.
That's no longer the case, though. In the last few months/years (not
sure since when) we're getting sporadic failures because the test
doesn't wait for the request to finish before making another request
with the `visit` method. This sometimes results in flaky tests.
Some of these tests have recently failed in our CI. Here are a few
examples (note the numbers don't follow an order because these tests
failed in different jobs):
```
1) Admin edit translatable records Current locale translation does not
exist For ActivePoll Shows first available fallback
Failure/Error: expect(page).to have_content "Sondage en Français"
expected to find text "Sondage en Français" in "Language: \n
\nEnglish\nDeutsch\nEspañol\nFrançais\nNederlands\nPortuguês
brasileiro\n中文\n Go back to CONSUL DEMOCRACY\nCONSUL
DEMOCRACY\nADMINISTRATION\nMenu\nNotifications\nMy content\nMy
account\nSign out\nProposals\nDebates\nComments\nPolls\n
Collaborative Legislation\nParticipatory budgets\nVoting booths
\nSignature Sheets\nMessages to users\nSite content\nModerated
content\nProfiles\nStatistics\nSettings\nProposals dashboard\n×
\nPolls description updated successfully.\nList of polls\nPolls
description\nCreate poll\nThere are no polls."
2) Public area translatable records Existing records Update a
translation With valid data Changes the existing translation
Failure/Error: expect(page).to have_field "Debate title",
with: "Title in English"
expected to find field "Debate title" that is not disabled but
there were no matches
2) Admin collaborative legislation Update Edit milestones summary
Failure/Error: expect(page).to have_content "There is still a long
journey ahead of us"
expected to find text "There is still a long journey ahead of us"
in "Language: \n
\nEnglish\nDeutsch\nEspañol\nFrançais\nNederlands\nPortuguês
brasileiro\n中文\n Go back to CONSUL DEMOCRACY\nCONSUL
DEMOCRACY\nADMINISTRATION\nMenu\nNotifications\nMy content\nMy
account\nSign out\nProposals\nDebates\nComments\nPolls\n
Collaborative Legislation\nParticipatory budgets\nVoting booths
\nSignature Sheets\nMessages to users\nSite content\nModerated
content\nProfiles\nStatistics\nSettings\nProposals dashboard\n×
\nProcess updated successfully. Click to visit\nBack\nAn example
legislation process\nInformation\nHomepage\nDebate\nProposals\n
Drafting\nFollowing\n1 language in use\nCurrent language\n
English\nSummary\n Format\n ◢\n Milestone\nManage progress
bars\nDon't have defined milestones\nCreate new milestone".
(However, it was found 1 time including non-visible text.)
3) Admin collaborative legislation SDG related list create Collaborative
Legislation with sdg related list
Failure/Error:
within("tr", text: "Legislation process with SDG related content") do
expect(page).to have_css "td", exact_text: "17"
end
Capybara::ElementNotFound:
Unable to find css "tr"
4) Valuation budget investments Valuate Feasibility can be marked as
pending
Failure/Error: expect(find("#budget_investment_feasibility_undecided"))
.not_to be_checked
Capybara::ElementNotFound:
Unable to find css "#budget_investment_feasibility_undecided"
3) Custom information texts Show custom texts instead of default ones
Failure/Error:
within("#section_help") do
expect(page).to have_content "Custom help with debates"
expect(page).not_to have_content "Help with debates"
end
4) Admin budgets Update Deselect all selected staff
Failure/Error: expect(page).to have_link "Select administrators"
expected to find link "Select administrators" but there were no
matches
3) Admin polls SDG related list edit poll with sdg related list
Failure/Error:
within("tr", text: "Upcoming poll with SDG related content") do
expect(page).to have_css "td", exact_text: "17"
end
Capybara::ElementNotFound:
Unable to find css "tr"
4) Admin polls SDG related list create poll with sdg related list
Failure/Error:
within("tr", text: "Upcoming poll with SDG related content") do
expect(page).to have_css "td", exact_text: "17"
end
Capybara::ElementNotFound:
Unable to find css "tr"
5) Admin custom images Image is replaced on admin newsletters
Failure/Error:
within(".newsletter-body-content") do
expect(page).to have_css("img[src*='logo_email_custom.png']")
end
Capybara::ElementNotFound:
Unable to find css ".newsletter-body-content"
6) Admin custom images Image is replaced on front views
Failure/Error:
within("#map") do
expect(page).to
have_css("img[src*='custom_map.jpg'][alt='Districts list']")
end
Capybara::ElementNotFound:
Unable to find css "#map"
```
The absence of labels in these controls made them hard to use,
particularly for people who use screen readers.
Note we're removing the "Choose language" prompt, since we always
automatically choose a language and not choosing a language doesn't
really make sense. The only scenario where the prompt was used took
place when all languages had been removed but, in that case, the "Choose
language" prompt was misleading because there were no languages to
choose from.
As mentioned in commits 5311daadf and bb958daf0, using links combined
with JavaScript to generate POST (or, in this case, DELETE) requests to
the server has a few issues.
The link to edit the process is already present before clicking the
"All" link, which meant the test failed sometimes because Capybara might
try to click on the "Edit" link at the same time the page is changing
due to the click on the "All" link".
Due to this issue, this test has failed at least one in our CI [1].
[1] https://github.com/consul/consul/runs/2324773853
JavaScript is used by about 98% of web users, so by testing without it
enabled, we're only testing that the application works for a very
reduced number of users.
We proceeded this way in the past because CONSUL started using Rails 4.2
and truncating the database between JavaScript tests with database
cleaner, which made these tests terribly slow.
When we upgraded to Rails 5.1 and introduced system tests, we started
using database transactions in JavaScript tests, making these tests much
faster. So now we can use JavaScript tests everywhere without critically
slowing down our test suite.
System tests are used to test the application from the user's point of
view. To test for specific exceptions, particularly regarding
authorization permissions, controller tests fit better.
Another option would be to test the page displayed shows a certain text,
like "Internal server error". I'm choosing controller tests because
they're faster and we're basically testing the same scenario many times
and we've already got a test checking what happens when users access a
page raising an exception.
We were repeating the same code over and over (with a few variants) to
setup tests which require an administrator. We can use a tag and
simplify the code.
In some tables, we had "actions", and some columns were also links
pointing to some places. Having both of them at the same time is
confusing, particularly since traditionally the links in the columns
pointed to the same place as some of the actions (although that's not
the case since commit 48db31cd).
We're still keeping links in tables which don't have an action column.
For instance, the proposals table has a "select" button which would be
harder to use if we had action buttons next to it.
