Commit Graph

9 Commits

Author SHA1 Message Date
Javi Martín
23945c2a7c Use JavaScript in tests dealing with tabs
So what we write in the tests is close to what users experience.
2021-04-07 14:41:06 +02:00
Javi Martín
5612ada2d4 Remove unused variables 2021-04-07 14:35:30 +02:00
Javi Martín
8a7555092f Explicitly use rack driver in dashboard test
There seems to be a bug in the dashboard when calculating
`accumulat_supports` because in some cases `vote_weight` is `nil`.

Besides, these tests check the database after the browser has started,
instead of checking things from a user's point of view.

Since both the tests and the code are wrong, and I'm not familiar enough
with the code in the dashboard section, for now I'm leaving things as
they were.
2021-04-07 14:35:30 +02:00
Javi Martín
fb88e0b77c Fix number of new notifications
We were displaying the total number of notifications with a message "You
have N unread notifications", but were using the total number of
notifications instead of the unread ones.
2021-02-18 14:45:48 +01:00
Javi Martín
4c289f2489 Simplify notification item HTML
Other than simplifying the view, we can write tests using `click_link`,
which makes the tests more robust. Clicking the `.icon-notification`
element was causing some tests to fail when defining a window height of
750 pixels in the `admin_budgets` branch.
2021-02-16 23:21:51 +01:00
Javi Martín
d8f1c462de Try to avoid PG::ProtocolViolation error in tests
While running our test suite, we were getting an exception sometimes:

```
Proposal Notifications In-app notifications from the proposal's author Followers should receive a notification
   Failure/Error: notification_for_user2 = Notification.find_by(user: user2)
      ActiveRecord::StatementInvalid:
        PG::ProtocolViolation: ERROR:  bind message supplies 0 parameters, but prepared statement "" requires 2
        : SELECT  "notifications".* FROM "notifications" WHERE "notifications"."user_id" = $1 LIMIT $2
   # ./spec/system/proposal_notifications_spec.rb:268
```

Sometimes we were getting a similar exception in a different test:

```
Commenting legislation questions Merged comment threads Reply on a multiple annotation thread and display it in the single annotation thread
And sometimes we were getting a different one:
   Failure/Error: annotation.comments.roots.sort_by_most_voted.limit(Legislation::Annotation::COMMENTS_PAGE_SIZE).each do |comment|
     ActionView::Template::Error:
       PG::ProtocolViolation: ERROR:  bind message supplies 0 parameters, but prepared statement "" requires 3
```

My best (wild) guess is these exceptions might take place because the
test is accessing the database and at the same time the browser
(chromedriver) process is accessing the database, with code like:

```
 find(".icon-notification").click
 notification_for_user2 = Notification.find_by(user: user2)
```

Or:

```
first(:css, ".annotator-hl").click
(...)
comment = annotation1.comments.first
click_link "Reply"
```

This behavior happened sometimes while using transactional fixtures and
a shared database connection with feature specs (before system specs
were introduced in Rails 5.1) when some queries were triggered from the
test after the browser process was started.

So we're avoiding the situation by writing the tests from the user's
point of view. This is just an attempt at fixing the issue; I don't know
whether these changes will fix it since I've only seen this exception on
Github Actions (never on my machine). Worst case scenario, we're still
improving the tests legibililty.
2021-01-22 12:31:48 +01:00
Javi Martín
25facc112f Remove double requests
They were added in commit bb4e4dc084; although I must admit I don't
know the reason why they were needed.
2021-01-22 12:26:59 +01:00
Javi Martín
c255d1b91c Avoid seasonal clock changes issues in specs
Using `travel` we go to `Time.now + interval`. If the application's time
zone changes due to seasonal clock changes during that interval, we
might travel to a time which is not the time we intended to travel to.

Example:

On a system using the UTC time zone, it's 9AM on October 25 (Friday).
Since by default CONSUL uses the Madrid time zone, it means the
application's time is 11AM.

We use `travel` to advance three days. That means we go to 9AM UTC on
October 28 (Monday). The application's time will be 10AM due to the
seasonal clock change, so we haven't fully traveled three days in
application's time.

To reproduce locally, run:

```
TZ=UTC rspec spec/system/proposal_notifications_spec.rb:410
```

Using `travel_to` with `3.days.from_now`, which uses the application's
time zone and so it will travel to October 28 at 11AM, solves the
problem.
2020-10-22 12:01:47 +02:00
Javi Martín
9427f01442 Use system specs instead of feature specs
We get rid of database cleaner, and JavaScript tests are faster because
between tests we now rollback transactions instead of truncating the
database.
2020-04-24 15:43:54 +02:00