These blocks are no longer used:
* `allowed_phase_list` isn't used since commit 04605d5d5
* `level_two_user` isn't used since commit 26d14cbd0
* `heading` in `budgets/stats_spec` was added in c2457e36a but never
used
* `translatable` was added in 44d137a4c but it's overwritten in all the
contexts.
* `annotation` isn't used since commit 79d00e7b9
* `admin` in `tags/budget_investments_spec` isn't used since 8a2e15980
* `budget` in `welcome_spec` was added in 87be6f302 but never used
I don't think this feature it was ever used. It was introduced in commit
49dec6061 as part of a feature that was removed in commits 1cd47da9d and
c45a0bd8ac.
We removed the link to this page in commit 83e8d6035 because poll
questions don't really make sense without a poll.
However, this page also contained information about successful
proposals, which might be interesting so administrators don't have to
navigate to the public area in order to find and create questions based
on successful proposals.
So we're keeping the part about successful proposals and linking it from
the proposals part of the admin area.
Note we're using translation keys like `successful_proposals_tab`, which
don't make sense anymore, for the successful proposals. We're doing so
because we've already got translations for these keys and, if we renamed
them, we'd lose the existing translations and our translators would have
to add them again.
Also note we're changing one poll question test a little bit so we
create the question from a successful proposal using the new page. There
are other tests checking how to create a question from the
admin/proposals#show action and other tests checking what happens when
accessing a successful proposal in the admin section, so we don't lose
any test coverage by changing an existing test instead of adding a new
one.
Finally, note that we've removing the `search` method in poll question
because we no longer use it. This currently makes the
`author_visible_name` database column useless; we aren't removing it
right now because we don't want to risk a possible data loss in a patch
release (we're about to release version 2.3.1), but we might remove it
in the future.
These rules aren't used since commit c5c9efee1.
Note that there's still an `icon-budget` element in the
`_investment_show` partial. The rules for this icon are already defined
inside the rules for the `.budget-investment-show` selector.
The `icon-budget` and `icon-proposals` HTML classes are still used in
the "Following" tab (the code uses the `followable_icon` method), but in
this case the `.budget-investment` or `.proposal` selectors aren't
present, so the properties weren't applied here either.
Similarly, there are elements with the `icon-debates` and
`icon-proposals` HTML classes in the dashboard area, but they aren't
inside elements matching the `.debate` or `.proposal` selectors either.
And there's an element with the `icon-debate` HTML class in the
legislation area. Once again, it isn't inside an element matching the
`.debate` selector.
So can safely remove this code.
This column isn't used since commit 4c0deb0ec because administrators can
associate videos to the answers since commit 5862eea51. The value of
this attribute isn't used in the public area since commit 8277e3cc2.
I accidentally made this mistake while trying to avoid the exact same
issue back in commit 73992b2c8. Accessing the database in a test after
starting the process running the browser has caused database corruption
in our CI multiple times.
This parameter isn't used since commit b4a6f664b.
Note we're changing the tests to use proposals instead of debates
because proposals may have images attached, while debates may not.
This way we know for sure the page has finished refreshing.
Note that, for now, we aren't adding a check after every call to the
`refresh` method because sometimes the page remains identical after
refreshing. Not sure what we should do in these cases.
After a `visit`, we were checking for content or filling in fields that
were already there before the `visit`, so we weren't 100% sure that the
request had finished before the test continued.
In the case of the verification tests, we were clicking the submit
buttons over and over without and then checking or interacting with
elements that were already there. Even though the button was disabled
between requests, meaning there wouldn't be simultaneous requests, it
was possible to interact with a form field before waiting for the
request to finish.
Some of these tests have recently failed on our CI, and it might be
because of that:
```
1) Admin budgets Edit Changing name for current locale will update the
slug if budget is in draft phase
Failure/Error: raise ex, cause: cause
Selenium::WebDriver::Error::UnknownError:
unknown error: unhandled inspector error: {"code":-32000,
"message":"Node with given id does not belong to the document"}
(Session info: chrome=134.0.6998.35)
1) Budgets creation wizard Creation of a multiple-headings budget by
steps
Failure/Error: expect(page).to have_content "Heading created
successfully!"
Selenium::WebDriver::Error::UnknownError:
unknown error: unhandled inspector error: {"code":-32000,
"message":"Node with given id does not belong to the document"}
(Session info: chrome=134.0.6998.35)
1) Custom information texts Show custom texts instead of default ones
Failure/Error: raise ex, cause: cause
Selenium::WebDriver::Error::UnknownError:
unknown error: unhandled inspector error: {"code":-32000,
"message":"Node with given id does not belong to the document"}
(Session info: chrome=134.0.6998.35)
1) Users Regular authentication Sign in Avoid username-email collisions
Failure/Error: raise ex, cause: cause
Selenium::WebDriver::Error::UnknownError:
unknown error: unhandled inspector error: {"code":-32000,
"message":"Node with given id does not belong to the document"}
(Session info: chrome=134.0.6998.35)
2) Verify Letter Code verification 6 tries allowed
Failure/Error: raise ex, cause: cause
Selenium::WebDriver::Error::UnknownError:
unknown error: unhandled inspector error: {"code":-32000,
"message":"Node with given id does not belong to the document"}
(Session info: chrome=134.0.6998.35)
2) Valuation budget investments Valuate Finish valuation
Failure/Error: raise ex, cause: cause
Selenium::WebDriver::Error::UnknownError:
unknown error: unhandled inspector error: {"code":-32000,
"message":"Node with given id does not belong to the document"}
(Session info: chrome=134.0.6998.35)
1) Users Delete a level 2 user account from document verification page
Failure/Error: raise ex, cause: cause
Selenium::WebDriver::Error::UnknownError:
unknown error: unhandled inspector error: {"code":-32000,
"message":"Node with given id does not belong to the document"}
(Session info: chrome=134.0.6998.35)
```
We're already testing the navigation in one test, and in some of these
tests we were checking, for instance, that the title of an investment is
present after a click, but since it is also present before that click,
that could lead to the test finishing before the request does.
This way it's more obvious what's going on.
Note that, in this case, the expectations were **not** true before
visiting the page, so we aren't fixing a flaky test.
This way the code is easier to follow, and we avoid having simultaneous
requests on different browser sessions, which in theory shouldn't affect
the application, but the test might be a bit more robust this way.
This way the tests are slightly faster, and we avoid a possible flaky
spec since in one test we were checking that a certain text was not
present, which didn't guarantee that the request to delete the document
had finished.
After clicking the "Save changes" button, we were sometimes checking
expectations that were already true before clicking the button, so it
was possible that the request generated by that button didn't finish
before the test did.
So now we're checking that the button is no longer present, which is
only true after the request has finished.
The expectation that there isn't a link with the text "Español" wasn't
updated in commit f7417d647, when the tests were updated after switching
the user interface from a list of links to a select field.
We're also adding an additional expectation after the `visit` to make
sure the request has finished before selecting English from the selector
(which probably doesn't make sense because it's the only option, but
changing it is out of the scope of this commit).
We were checking that, after a `visit`, we're redirected to the account
page, but, since we were already in the account page, the page doesn't
change at all. In order to be 100% that the request caused by the call
to `visit` has finished before checking the expectations, we're visiting
a different page first.
We weren't checking whether the request creating the proposal had
finished before checking the document. That's probably why this test has
recently failed on our CI:
```
1) Documents Metadata download document without metadata
Failure/Error: io = URI.parse(
"#{app_host}#{polymorphic_path(
Document.last.attachment)}"
).open
NoMethodError:
undefined method `attachment' for nil:NilClass
```
We weren't checking that the requests had finished before checking the
last sent email. That's probably why one of these tests has recently
failed on our CI:
```
1) System Emails Preview Pending #send_pending
Failure/Error: email = open_last_email
RuntimeError:
No email has been sent!
```
These tests don't use the browser to send emails since commit e21588ec1.
However, note that this commit actually actually decreased our test
coverage somehow; since then, we're no longer testing whether we send an
email to the author after clicking the "Publish comment" button. We
might need to add a test for this in the `spec/system/comments_spec.rb`
file... but that's a story for another time.
Note we need to stub the `deliver_later` method; otherwise,
`open_last_email` would raise an exception since no email would have
been delivered.
We're moving them now because these tests use the `open_last_email`
method, and we're looking for places where using this method might
result in a flaky test when used inside a system test.
These tests were testing mailer methods, but were inside a file for
system tests.
We're moving them now because these tests use the `open_last_email`
method, and we're looking for places where using this method might
result in a flaky test when used inside a system test.
These tests were testing model methods, but were inside files for system
tests.
We're moving them now because these tests use the `open_last_email`
method, and we're looking for places where using this method might
result in a flaky test when used inside a system test.
This way it's more clear what the user experience is during the process.
Note this did not result in flaky tests, since the tests continue by
clicking links, that are only present after the request has finished.
However, we're adding expectations so we don't have to think whether the
tests could be flaky and because this way we're also testing the user
experience; it would be strange for a user if they were redirected to a
page without a flash message.
Quoting from commit 57bda006b5:
> When clicking the button "Search", the link "newest" is already
> present, so capybara might click the "newest" link before the "Search"
> request is finished, leading to unexpected results.
>
> Checking the page to make sure the "Search" request has finished
> before clicking the "newest" link solves the problem.
We're doing the same thing for the other tests that click the "Search"
button and then clicked on a link. We're also making sure the language
selector changes before clicking it again.
In most tests calling this method, we were doing another visit right
after calling this method, so by removing this `visit` call we're making
the tests slightly faster and easier to follow.
We weren't checking that the request caused by clicking on the "Send
instructions" button had finished before continuing with the test.
Perhaps that's why this test has recently failed on our CI:
```
3) Emails Reset password
Failure/Error: email = open_last_email
RuntimeError:
No email has been sent!
```
We're also adding an expectation to the `login_as_manager` method and
the methods to submit proposal and investment forms to make sure that,
when these method finish, the request finishes as well.
This way we're 100% sure we won't have simultaneous requests since the
test will wait for one request to finish before continuing.
In the debates spec, we're also making sure the expectations between the
two consecutive visits are different.
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"
```
Our tests are failing fairly consistently on CI. We
believe that this started when the most recent Github Actions
ubuntu-latest image was released[1]. Though, we don't 100% understand
why. There's some speculation[2] that the root of the issue has
something to do with Chrome/Chromedriver version 134 (which is the
version bundled with that new ubuntu-latest).
[1]: https://github.com/actions/runner-images PR#11761
[2]: https://github.com/teamcapybara/capybara Issue#2800
We've largely copied what the signon team did to get
their CI back up-and-running[^3].
[^3]: https://github.com/alphagov/signon PR#3663
In this commit we remove the version of Chrome that the
actions/runner-images image bundles for us, so that Selenium
doesn't try to use it and install an old version of Chrome and
Chromedriver.
We update the texts in Spanish ourselves, just like we update the
English texts. However, every time we get a pull request to update
translations from Crowdin, we get a bunch of texts in Spanish that we
ignore.
Hopefully excluding the Spanish language will solve the issue.
This way we're also checking mistakes like closing tags that don't match
their opening element, which we detected by manually running HTML
Beautifier with the `-e` option, and fixed two commits ago.
Note there was a false positive in the mailer layout. We don't know the
cause. Maybe closing the ERB tag right before the HTML opening tag and
the lack of other attributes on the tag made HTML Beautifier think the
tag wasn't correctly open, but on the other hand, we have the exact same
line in other layouts where HTML Beautifier works fine. We're fixing it
by adding an HTML id attribute to the element.
We had inconsistent indentation in many places. Now we're fixing them
and adding a linter to our CI so we don't accidentally introduce
inconsistent indentations again.
In some places, we accidentally opened the same tag twice instead of
closing it, while in some other places we closed a tag without opening
it in the first place.
We've detected these issues thanks to the HTML Beautifier gem, which
we're about to start using for indentation purposes.
Using an `if..else` block made the code harder to follow since the
opening tag was inside the block but the closing tag was outside it.
Moreover, it didn't work well with HTML Beautifier (a gem we're going to
introduce to manage ERB indentations).
