Skip to content

Commit 8538527

Browse files
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.rb
1 parent 7cf9faf commit 8538527

File tree

3 files changed

+9
-7
lines changed

3 files changed

+9
-7
lines changed

config/environments/test.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@
6262
# Annotate rendered view with file names.
6363
# config.action_view.annotate_rendered_view_with_filenames = true
6464

65+
config.active_job.queue_adapter = :test
66+
6567
# bullet - N+1
6668
config.after_initialize do
6769
Bullet.enable = true

spec/concepts/school_student/create_batch_spec.rb

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,6 @@
2222
]
2323
end
2424

25-
before do
26-
ActiveJob::Base.queue_adapter = :test
27-
end
28-
2925
context 'when queuing a job' do
3026
before do
3127
stub_profile_api_create_school_students(user_ids: [SecureRandom.uuid, SecureRandom.uuid])

spec/jobs/create_students_job_spec.rb

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,20 @@
1717
}]
1818
end
1919

20-
before do
20+
around do |example|
21+
original = ActiveJob::Base.queue_adapter
2122
ActiveJob::Base.queue_adapter = :good_job
23+
example.run
24+
ensure
25+
ActiveJob::Base.queue_adapter = original
26+
end
2227

28+
before do
2329
stub_profile_api_create_school_students(user_ids: [user_id])
2430
end
2531

2632
after do
2733
GoodJob::Job.delete_all
28-
29-
ActiveJob::Base.queue_adapter = :test
3034
end
3135

3236
it 'calls ProfileApiClient' do

0 commit comments

Comments
 (0)