We were doing it most of the time, but in some cases we were clicking
the "Sign out" link instead. These actions aren't the same, just like
using `login_as` is not the same as visiting the sign in page and
submitting the form.
Some of these tests failed sometimes because the user wasn't signed in
after using `login_as`. One possible cause could be that we weren't
adding an expectation after clicking the "Sign out" link.
So using `logout` adds consistency, simplifies the code, and might
reduce the chance of these tests failing in the future (although they
might still fail in the future because some of these tests check the
database after a `visit` call).
Having a class named `Poll::Question::Answer` and another class named
`Poll::Answer` was so confusing that no developer working on the project
has ever been capable of remembering which is which for more than a few
seconds.
Furthermore, we're planning to add open answers to polls, and we might
add a reference from the `poll_answers` table to the
`poll_question_answers` table to property differentiate between open
answers and closed answers. Having yet another thing named answer would
be more than what our brains can handle (we know it because we did this
once in a prototype).
So we're renaming `Poll::Question::Answer` to `Poll::Question::Option`.
Hopefully that'll make it easier to remember. The name is also (more or
less) consistent with the `Legislation::QuestionOption` class, which is
similar.
We aren't changing the table or columns names for now in order to avoid
possible issues when upgrading (old code running with the new database
tables/columns after running the migrations but before deployment has
finished, for instance). We might do it in the future.
I've tried not to change the internationalization keys either so
existing translations would still be valid. However, since we have to
change the keys in `activerecord.yml` so methods like
`human_attribute_name` keep working, I'm also changing them in places
where similar keys were used (like `poll_question_answer` or
`poll/question/answer`).
Note that it isn't clear whether we should use `option` or
`question_option` in some cases. In order to keep things simple, we're
using `option` where we were using `answer` and `question_option` where
we were using `question_answer`.
Also note we're adding tests for the admin menu component, since at
first I forgot to change the `answers` reference there and all tests
passed.
Note we're excluding a few files:
* Configuration files that weren't generated by us
* Migration files that weren't generated by us
* The Gemfile, since it includes an important comment that must be on
the same line as the gem declaration
* The Budget::Stats class, since the heading statistics are a mess and
having shorter lines would require a lot of refactoring
This test will fail if the ID of the created record is the same as the
ID of the first user we create in the test. The chance is very low due
to the `rand(9999999)` which causes the test to fail just once every ten
million times. However, why assigning the ID in the first place? Without
it, the test will never fail due to conflicting IDs.
Checking the database with methods like Activity.last does not test that
the record is present where it should be (first record of the table in
this case). In these tests there's only one record, though, so the order
doesn't matter that match.
However, calling methods like Activity.last generates a database query
after the process running the browser has been started, and this might
lead to inconsistent data.
Changing the database after the process running the browser has started
is proving to be one of the reasons tests are failing sometimes, so
we're reducing the number of times were that happens. In this case, we
were changing a setting.
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.
By using real XML responses developers will be able to understand better
how the integration works (the data flow), and the correspondency between
`remote_census` settings and their place at a real XML response.
As `stubbed_responses` methods were removed from the model layer now the
stubbing part should be managed from the test environment code so also
added a new helper module `RemoteCensusSetup` that can be used anywhere
where we need to call the web service.
Co-Authored-By: Javi Martín <javim@elretirao.net>
