Refactor/rewrite spec/services/notification_service_spec.rb
What is the productivity problem to solve?
This is an extremely costly, and large, spec that currently takes about 11.5 minutes to run on my laptop (even more on the CI I think), and performs around 300k Postgres queries. @stanhu has provided some relevant stats in create-stage#12604 (comment 263897284)
I think I understand the reason for such in-depth coverage in this spec, but I do think the benefits of the current coverage are now outweighed by just how slow it is.
Problem identification checklist
-
The root cause of the problem is identified. -
The surface of the problem is as small as possible.
What are the potential solutions?
Personally I think the coverage of this spec should be reduced. Currently it's responsible for not just testing the methods of the service, but also does a lot of checking of the exact recipients of each notification. I think the recipient coverage would perhaps be better placed in spec/services/notification_recipient_service_spec.rb, which is currently quite sparse. This would then allow us to rewrite the notification service spec to primarily cover the logic of whether an email is sent or not, and leave the "who" out, which would in theory let us completely remove database interaction from this spec and instead cover it with stubs/doubles.
-
All potential solutions are listed. -
A solution has been chosen for the first iteration: PUT THE CHOSEN SOLUTION HERE
Who and when will the solution be implemented?
Verify that the solution has improved the situation
-
The solution improved the situation. - If yes, check this box and close the issue. Well done!
🎉 - Otherwise, create a new "Productivity Improvement" issue. You can re-use the description from this issue, but obviously another solution should be chosen this time.
- If yes, check this box and close the issue. Well done!