In this blog post, I will talk about how I found and fixed a flaky test in our RSpec test suite, and how I wrote a custom RuboCop rule to prevent it from happening again.
DocSpring uses Stripe to handle credit card processing. We have a large test suite that covers lots of different billing flows and edge cases. We use Stripe's billing customer portal, which allows customers to update their credit card details and view invoices. Stripe hosts this portal for us, so we don't have to build all of this functionality ourselves.
I wrote an integration test that makes sure we can generate a link to this billing portal page. This test was "flaky", which means that it was randomly failing from time to time. Flaky tests can be really annoying, especially when they only fail once every few weeks. It's very difficult to fix something if you don't know how to make it happen consistently.
I figured out that this test failure was order-dependent, which means that it was only failing whenever another test ran before it. We run our RSpec tests in a random order, which helps us find issues like this. The problem was happening because we use the stripe-ruby-mock gem to mock requests to Stripe's API. This mocking behavior is enabled by calling StripeMock.start
, and turned off by calling StripeMock.stop
As you might have guessed by now, we had a test that wasn't tidying up after itself by calling StripeMock.stop
.
The stripe-ruby-mock docs suggest using it like this:
describe MyApp do
let(:stripe_helper) { StripeMock.create_test_helper }
before { StripeMock.start }
after { StripeMock.stop }
I had started with this approach, but it looks like I had accidentally moved the StripeMock.stop
call into a nested block. This means that only some of the tests in this spec file were calling StripeMock.stop
. If we got unlucky and tests were run in a different order, we'd end up with StripeMock
enabled for the rest of our test suite. This didn't actually cause too many problems, apart from this one billing portal test. This test makes a live request to Stripe's API, which we record/replay using VCR.
If you look at our test failure logs in CircleCI above, you'll notice StripeMock complaining that it doesn't know about the post /v1/billing_portal/sessions
API endpoint. This is how I figured out the issue, because I realized that StripeMock shouldn't have been running at all.
The solution was to make sure that our tests always call StripeMock.stop
whenever they call StripeMock.start
. But maybe there's a better way to do this, instead of using separate before
and after
blocks?
We can actually group these calls together by using an around
block, like this:
around(:each) do |ex|
StripeMock.start
ex.run
StripeMock.stop
end
This around
block will start StripeMock, run the test, and then stop StripeMock once the test is finished. This convention keeps the #start
and #stop
calls right next to each other, so it helps to prevent us from accidentally removing the #stop
call or moving it somewhere else during a refactor.
Now that I've fixed the flaky test, I also want to make sure that it never happens again. I want to prevent myself (and other developers on our team) from calling StripeMock.start
in a before
block, and only allow it inside an around
block.
We use RuboCop, which is a Ruby static code analyzer (a.k.a. linter
) and code formatter. RuboCop comes with many cops that can check your code for style, formatting, security issues, code complexity, among many other things. Many gems also have their own set of RuboCop rules. We currently use the RuboCop extensions for:
- Rails
- Rake
- RSpec
- Capybara
- FactoryBot
I decided to try writing a custom RuboCop rule (or "cop") that would scan our RSpec test files for StripeMock.start
. I would like it to warn us whenever we use it in a before
block instead of an around
block.
Here's the code I wrote for our custom cop (with some help from ChatGPT!) I saved this file at lib/rubocop/cop/stripe_mock_usage.rb
# frozen_string_literal: true
require 'rubocop'
# This cop checks that StripeMock.start and StripeMock.stop are only used within an around block (instead of before/after.)
# This prevents us from accidentally starting StripeMock in a before(:all) or before(:each) block and
# forgetting to stop it, which can cause flaky tests.
module RuboCop
module Cop
module RSpec
class StripeMockUsage < Cop
MSG = 'Only use `StripeMock.start` and `StripeMock.stop` within an `around(:each)` block.'
def_node_search :stripe_mock_start?, <<~PATTERN
(send (const nil? :StripeMock) :start)
PATTERN
def_node_search :stripe_mock_stop?, <<~PATTERN
(send (const nil? :StripeMock) :stop)
PATTERN
def on_block(node)
return unless %i[before after].include? node.send_node.method_name
# Search within the before/after block for StripeMock.start and StripeMock.stop calls
node.each_descendant(:send) do |send_node|
if stripe_mock_start?(send_node) || stripe_mock_stop?(send_node)
add_offense(send_node, message: MSG)
end
end
end
end
end
end
end
See the "Development" page in the RuboCop docs for more information on writing new cops.
I then enabled this custom cop by adding the following to our .rubocop.yml
:
require:
- ./lib/rubocop/cop/stripe_mock_usage.rb
RSpec/StripeMockUsage:
Enabled: true
Include:
- 'spec/**/*_spec.rb'
Now we get a warning whenever we call StripeMock.start
or StripeMock.stop
in a before
or after
block:
It's not a perfect check, but I think this could be a nice reminder. My personal philosophy is that I try to fully stamp out any flaky tests, bugs, or security issues so that they can never happen again. I haven't truly fixed an issue unless I've also added a new test or linting rule.
I enjoyed learning more about the Ruby AST and how RuboCop works internally. I have a few other ideas for cops that might be helpful, so I might be writing some more custom cops in the future. Thanks for reading!