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.

Sign up for GitHub

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

Jump to bottom

Simplify after blocks in specs #3702

Merged
merged 4 commits into from
Sep 23, 2019
Merged

Simplify after blocks in specs #3702

merged 4 commits into from
Sep 23, 2019

Commits on Sep 23, 2019

  1. Remove redundant setting resets in after blocks 

    Settings are stored in the database, and so any changes to the settings
done during the tests are automatically rolled back between one test and
the next one.

There were also a few places where we weren't using an `after` block but
changing the setting at the end of the test.
    @javierm
    javierm committed Sep 23, 2019
    Configuration menu
    Copy the full SHA
    da121eb View commit details
    Browse the repository at this point in the history

  2. Remove redundant I18n resets in after blocks 

    We already configure `I18n.locale` and we reset Globalize's fallbacks
before every test.

On the other hand, RSpec automatically resets anything which is stub
with `allow`, so there's no need to use `and_call_original` in an
`after` block.
    @javierm
    javierm committed Sep 23, 2019
    Configuration menu
    Copy the full SHA
    7be72df View commit details
    Browse the repository at this point in the history

  3. Simplify tests using delayed_job 

    Among other advantages, now we can run these tests with
`rspec --tag delayed_jobs`.
    @javierm
    javierm committed Sep 23, 2019
    Configuration menu
    Copy the full SHA
    eb7a052 View commit details
    Browse the repository at this point in the history

  4. Remove redundant feature settings enabled in tests 

    These settings are enabled by default.

It could be argued explicitely enabling the features makes tests more
consistent, because they'll work if we change the default setting. It
could also be argued that it makes tests more expressive because it
makes the reader realize certain things will only work if a setting is
enabled.

However, we were only doing so in a few tests. The truth is, thousands
of our tests depend on certain features being enabled. So IMHO we should
be consistent and either set them on every test, or not at all. I'm
choosing the latter option for simplicity.
    @javierm
    javierm committed Sep 23, 2019
    Configuration menu
    Copy the full SHA
    2ec176a View commit details
    Browse the repository at this point in the history