Bug Tales: Trust nothing but the real thing
Javier Acero
on
We use our Slack application constantly to check domain availability and register domains. Truth be told, I don't think any of us has registered a domain from the website in a long time. Tiny P makes it way funnier than using the web.
Several weeks ago we noticed that some information was missing when a domain was registered. Both the domain name and the expiration date were not showing up once the registration completed:
We had just released the stable v2 version of our API so we immediately realized that we needed to update the Slack app to use the latest DNSimple API v2 Ruby client (yes, our Slack integration is written in Ruby).
This is what we did to fix it:
- We verified that we could reproduce the issue on our "sandbox" Slack application.
- We created an issue in the corresponding GitHub repo.
- We updated the
dnsimpleRuby Gem to the latest version. - We run all the specs. All passed.
- We pushed the new version to the "sandbox" Slack application.
- We verified that the error was no longer happening.
Quick review of the corresponding Pull Request, merge, and deploy of the new version. All good.
Too good to be true
Unfortunately the client update had introduced a bug in a totally different part of the app, a quite important one: the sign up. We noticed as soon as an exception notification came in from Bugsnag.
You may be wondering what went wrong if all tests were green. Yes, they were definitely green, but they weren't using the actual DNSimple API.
Trust nothing but the real thing
The Slack app has a thin wrapper around the DNSimple client. This object has a double function: it protects us from changes in the client interface and exposes a more convenient API for us to be used by the different Slack commands.
This wrapper, and not the client itself, happens to be mocked in the tests (because, as the GOOS says, you only mock what you own), and doesn't have tests of its own (yes, yes, I know). This is why we didn't detect the bug on time.
The missing contract test
There is a corollary to only mock what you own: make sure your wrappers behave the way their clients expect them to. This is why you need contract tests.
Back when we were writing the app we didn't write these tests because we were constantly manually testing the app to see the actual output of the commands and adjust it. We were lazy and we paid the price.
We have now added contract tests for our wrapper. These tests are not part of the normal build, but we can run them when we consider necessary.
Hopefully they will prevent any other Bug Tales posts being about the Slack app.
Javier Acero
Programmer. Minimalist. Apple fanboy. Currently having fun at DNSimple. I love coffee and my cat.
We think domain management should be easy.
That's why we continue building DNSimple.
