Commit 8538527
committed
Default to ActiveJob's test queue adapter in specs
TL;DR This makes the behaviour of ActiveJob in specs much more
consistent and in doing so fixes a long-lived flakey spec.
Previously, the majority of the specs were using the `GoodJob::Adapter` [1]
in specs instead of the built-in `ActiveJob::TestHelper::TestQueueAdapter` [2].
This was because of the `config.active_job.queue_adapter = :good_job`
line in `config/application.rb` [3] which was not overridden in
`config/environments/test.rb` [4].
However, confusingly, the `GoodJob::Adapter` has an `execution_mode` [5]
which defaults to `inline` in the Rails test environment [6]. This means
that by default, jobs will be executed inline, i.e. synchronously.
Adding to the confusion, the `SchoolStudent::CreateBatch` spec had a
`before` block which set the adapter to `:test` [7], i.e.
`ActiveJob::TestHelper::TestQueueAdapter`, but then never set it back to
`:good_job`. Thus any specs which ran *after* this spec then used the
`:test` adapter rather than the `:good_job` adapter in `:inline`
execution mode.
Similarly, the `CreateStudentsJob` spec set the adapter to `:good_job`
in a `before` block and then to `:test` in an `after` block [8]. Thus,
again, any specs which ran *after* this spec then used the `:test`
adapter rather than the `:good_job` adapter in `:inline` mode.
When using the `:test` adapter, jobs that are enqueued, e.g. using
`ActiveJob::Enqueuing::ClassMethods#perform_later` [8] or
`ActionMailer::MessageDelivery#deliver_later` [9] will *not* be executed
inline, i.e. they will *not* be executed synchronously.
In combination this meant that depending on the order in which they ran
some specs used an adapter which executed jobs synchronously and some
used an adapter which executed jobs asynchronously (i.e. often not at
all). And this led to at least one intermittent spec failure [10] which
was hard to diagnose.
In this commit, I've tried to make things more consistent. I've chosen
to explicitly set the adapter to `:test` in `config/environments/test.rb`,
because a bunch of specs were already relying on it and I think it gives
the least surprising behaviour.
I've left the `CreateStudentsJob` spec using the `:good_job` adapter,
because it and the job implementation has some somewhat complex
GoodJob-related behaviour. However, I've replaced the code in the
`before` & `after` blocks with an `around` block which ensures the
adapter is set back to its default value, i.e. `:test` after each
example, so it doesn't accidentally get left in an unexpected state.
I've also checked that no other specs are relying on the use of the
`:good_job` adapter, e.g. making negative assertions, by temporarily
setting the adapter to `:good_job` in `config/environments/test.rb`.
These changes have the effect of fixing the intermittent spec failure
mentioned above:
This example [11] has been failing in the CI build intermittently for a
long time and it failed again in this CI build [10]. However, this time
the related example [12] I added recently [13] to help diagnose the
problem has also failed which gives us a bit more to go on:
1) SchoolTeacher::Invite does not return an error in operation response
Failure/Error: expect(response[:error]).to be_blank
expected `"Error inviting school teacher: key not found: \"EDITOR_PUBLIC_URL\"".blank?` to be truthy, got false
# ./spec/concepts/school_teacher/invite_spec.rb:21:in `block (2 levels) in <top (required)>'
I managed to reproduce this locally by removing the `EDITOR_PUBLIC_URL`
env var from my local `.env` file and running the example. The example
calls `SchoolTeacher::Invite.call` which calls
`TeacherInvitation.create!` which triggers
`TeacherInvitation#send_invitation_email` via an `after_create_commit`
callback. In turn this calls `InvitationMailer#invite_teacher` which
tries to render `app/views/invitation_mailer/invite_teacher.text.erb`
when `#deliver_later` is called.
However, this calls `ENV.fetch` with 'EDITOR_PUBLIC_URL' as the key [14]
which in this case is missing resulting in a `KeyError` exception being
raised. This exception is rescued in `SchoolTeacher::Invite.call` [15]
and an error is set on the `OperationResponse` instance which means that
the `expect(response.success?).to be(true)` assertion fails.
This problem is fixed by the changes in this commit, because the
`app/views/invitation_mailer/invite_teacher.text.erb` template is now no
longer ever rendered, because the job enqueued by `#deliver_later` is no
longer executed. I did consider fixing it by setting the
`EDITOR_PUBLIC_URL` env var in the spec, but we already have the
`InvitationMailer` spec [16] to test the rendering of the template.
I suspect most of us were not seeing the spec fail locally, because we
had the `EDITOR_PUBLIC_URL` env var set in our local `.env` file.
[1]: https://www.rubydoc.info/gems/good_job/4.3.0/GoodJob/Adapter
[2]: https://api.rubyonrails.org/v7.1.3.4/classes/ActiveJob/TestHelper/TestQueueAdapter.html
[3]: https://github.com/RaspberryPiFoundation/editor-api/blob/7cf9faf02696d7eb14afb3fe1b3e119f99ca983f/config/application.rb#L53
[4]: https://github.com/RaspberryPiFoundation/editor-api/blob/7cf9faf02696d7eb14afb3fe1b3e119f99ca983f/config/environments/test.rb
[5]: https://www.rubydoc.info/gems/good_job/4.3.0/GoodJob/Adapter#initialize-instance_method
[6]: https://github.com/bensheldon/good_job/blob/v4.3.0/README.md#write-tests
[7]: https://github.com/RaspberryPiFoundation/editor-api/blob/7cf9faf02696d7eb14afb3fe1b3e119f99ca983f/spec/concepts/school_student/create_batch_spec.rb#L25-L27
[8]: https://api.rubyonrails.org/v7.1.3.4/classes/ActiveJob/Enqueuing/ClassMethods.html#method-i-perform_later
[9]: https://api.rubyonrails.org/v7.1.3.4/classes/ActionMailer/MessageDelivery.html#method-i-deliver_later
[10]: https://app.circleci.com/pipelines/github/RaspberryPiFoundation/editor-api/2395/workflows/52739fa5-1ecb-492c-8b77-eab8e08093ed/jobs/4514
[11]: https://github.com/RaspberryPiFoundation/editor-api/blob/7cf9faf02696d7eb14afb3fe1b3e119f99ca983f/spec/concepts/school_teacher/invite_spec.rb#L14-L17
[12]: https://github.com/RaspberryPiFoundation/editor-api/blob/7cf9faf02696d7eb14afb3fe1b3e119f99ca983f/spec/concepts/school_teacher/invite_spec.rb#L19-L22
[13]: 61664b7
[14]: https://github.com/RaspberryPiFoundation/editor-api/blob/7cf9faf02696d7eb14afb3fe1b3e119f99ca983f/app/views/invitation_mailer/invite_teacher.text.erb#L9
[15]: https://github.com/RaspberryPiFoundation/editor-api/blob/7cf9faf02696d7eb14afb3fe1b3e119f99ca983f/lib/concepts/school_teacher/invite.rb#L10-L14
[16]: https://github.com/RaspberryPiFoundation/editor-api/blob/7cf9faf02696d7eb14afb3fe1b3e119f99ca983f/spec/mailers/invitation_mailer_spec.rb1 parent 7cf9faf commit 8538527
File tree
3 files changed
+9
-7
lines changed- config/environments
- spec
- concepts/school_student
- jobs
3 files changed
+9
-7
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
62 | 62 | | |
63 | 63 | | |
64 | 64 | | |
| 65 | + | |
| 66 | + | |
65 | 67 | | |
66 | 68 | | |
67 | 69 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
22 | 22 | | |
23 | 23 | | |
24 | 24 | | |
25 | | - | |
26 | | - | |
27 | | - | |
28 | | - | |
29 | 25 | | |
30 | 26 | | |
31 | 27 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
17 | 17 | | |
18 | 18 | | |
19 | 19 | | |
20 | | - | |
| 20 | + | |
| 21 | + | |
21 | 22 | | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
22 | 27 | | |
| 28 | + | |
23 | 29 | | |
24 | 30 | | |
25 | 31 | | |
26 | 32 | | |
27 | 33 | | |
28 | | - | |
29 | | - | |
30 | 34 | | |
31 | 35 | | |
32 | 36 | | |
| |||
0 commit comments