Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upstream #1568

Merged
merged 97 commits into from
Jul 20, 2018
Merged

Upstream #1568

merged 97 commits into from
Jul 20, 2018

Conversation

decabeza
Copy link

@decabeza decabeza commented Jul 16, 2018

References 🚀

This PR updates this repo with CONSUL one, including the following PR's:

Also in the commits list but are backports (already on AyuntamientoMadrid#master)

javierv and others added 30 commits June 23, 2018 19:58
There's a flaky test creating a group for a budget which takes place
because toggling a form using jQuery sometimes results in the button not
being correctly clicked by Capybara.

Checking the page with `expect(page).to have_button 'Create group'`
before clicking the button doesn't solve the problem; it looks like in
those cases Capybara waits for AJAX requests but not for JavaScript
animations.

See also issue consuldemocracy#2573.
Just like jQuery animations, they can make tests fail when using
Capybara.
With this name, it's easier to understand what it does.
The browser was generating one AJAX request per keystroke, ignoring the
timeout. The clearTimeout() function needs to be called with the ID
value returned by setTimeout().
As stated in #1196, compiling everything related to CKEditor made
compilations slower. However, not compiling any plugins meant Travis had
to compile them while running a test. It often resulted in a test
failing because the time Travis took to compile the plugins the
application uses exceeded Capybara's wait time.
The test "Budget Investments Show milestones" was failing in certain
cases where `Globalize.locale` had been changed in a previous test.

Since having different values in `Globalize.locale` and `I18n.locale`
has proven to be an issue on the test enviroment, this commit also
changes application code in order to avoid similar situations on
production.

See issue consuldemocracy#2718.
These tests were failing randomly because there is no guarantee
the methods `Budget#email_selected` and `Budget#email_unselected` will
always send the emails in the same order, because `investments.selected`
and `investments.unselected` don't necessarily return the records in the
order they were created.

Ordering by id is certainly not very elegant; however, ordering by
another field like `created_at` is dangerous because the record could be
created at (almost) the exact same time.

Related to issue consuldemocracy#2446 and issue consuldemocracy#2519.
There was a flaky spec supporting proposals. It's hard to reproduce and
be sure about what was going on, so here is my best guess. Given the
code:

```
within(".proposals-list") { click_link proposal.title }
within("#proposal_#{proposal.id}_votes") { click_link('Support') }
```

The first clicked link generates an AJAX request. Usually, Capybara
would wait for the AJAX request to generate a "Support" link in the
element `#proposal_XX_votes`. However, there's already a
`#proposal_XX_votes` element with a "Support" link on the proposals
index page!

So Capybara doesn't have to wait for the AJAX request to finish before
clicking the "Support" link. From then on, anything can happen.

Another option that works:

```
within(".proposals-list") { click_link proposal.title }
within(".proposal-show #proposal_#{proposal.id}_votes") { click_link('Support') }
```

With this code, the link selector fails on the proposals index page, and
Capybara waits for the AJAX request to finish.

Related to issue consuldemocracy#2558.
As explained by @iagirre with pinpoint accuracy [1]:

"If the officer_assignments are created at 23:59:59 and the rest of the
test is executed after 00:00:00, the dates for the objects and the
`Date.current` (used to check if there are any shifts today) won't be
the same, because the shift will be for, lets say, 07/03/2018 and
`Date.current` will be 08/03/2018, so, there are no shifts."

Freezing time avoids this issue.

Related to issues consuldemocracy#2520 and consuldemocracy#2521.

[1] #1342
It could have side effects (for example, a conflict after upgrading to
Rails 5).

Thanks @aitbw for the suggestion!
The tests depending on the date changing were still failing because
Date.current was being stubbed after loading the factories. The
following lines affected these specific tests:

factory :poll_officer_assignment, class: 'Poll::OfficerAssignment' do
(...)
  date Date.current
end

So if the tests were executed right before midnight, the sequence was:

1. The factories file was loaded, assigning Date.current to the date of
every Poll::OfficerAssignment to be created.
2. Time passed, so now it was after midnight.
3. The `travel_to` method freezed time, after midnight.
4. A Poll::OfficerAssignment factory was created, using the date it was
before midnight.

Using dynamic fixtures solves the problem:

factory :poll_officer_assignment, class: 'Poll::OfficerAssignment' do
(...)
  date { Date.current }
end

Now the sequence is:

1. The factories file is loaded, and since it finds a block, doesn't
assign a static value to every Poll::OfficerAssignment to be created.
2. Time passes, so now it's after midnight.
3. The `travel_to` method freezes time, after midnight.
4. A Poll::OfficerAssignment factory was created, and in executes the
block, using the current date, that is, after midnight.
decabeza and others added 23 commits July 15, 2018 21:28
Sorry, at work we have a co-worker (@rochgs) that thanks this "spaced" style
because of his visual handicap. Here you are!! ;)
…ages_on_dev_seeds

add proposal image on dev_seeds task
…r-recommendations

Debates and proposals recommendations for users
@decabeza decabeza changed the title [WIP] Upstream Upstream Jul 16, 2018
@decabeza decabeza mentioned this pull request Jul 19, 2018
@decabeza decabeza merged commit ef76908 into master Jul 20, 2018
@decabeza decabeza deleted the upstream branch July 20, 2018 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants