A funny thing happened last week. On Wednesday, we performed our weekly code deployment and released a handful of new features/bug fixes to the site. And then, about an hour later, someone on the team found this:
Notice the extra "asdf fdsa" in there? It's okay if you didn't, because neither did we. How did this happen? Don't you guys have a review process? I would have never let this happen on my project.
We do have a review process... and a lengthy one, which amongst other things helps us catch these types of oversights. But we're also human. Our process looks like this:
Our QA Process
- Developer writes code in their own feature branch.
- Devleoper reviews own code before marking ticket as "resolved".
- Another team member does both a code and functional review of the proposed resolution before merging the feature branch into the QA branch.
- As a team, we do a second round of QA on the tagged and ready to release code with all changes merged together to make sure nothing is out of place.
- As a team, we do QA on all critical paths on our site via a combination of automated and manual tests. Even if we didn't touch something, this ensures we didn't accidently break it.
- Finally, we tag and deploy a new version of the site.
In this latest case, however, here's what I think happened. The developer working on the issue added the text to do some testing, fixed that part of the issue, and moved on. After completing the rest of the tasks in the issue, they used features to update the in-code version of the view they were working on and comitted their change. During the two code reviews that happened, the changes to the view were missed because we read through the diff, assumed Features did what it was supposed to, and only casually glanced at the output before merging it to QA. These diffs from the Features module can be a real pain to figure out:
On the QA server, we didn't actually have any lessons in any series that were marked as "coming soon". Those had just been added to production that morning and our build server had made the build for QA in the middle of the night prior. So the issue didn't show up in a visual QA either, since this debug code was not related to a new feature that got added as part of this release, isn't in our critical paths, and therefore didn't have a specific QA task.
And thus this little bit of fairly harmful debug code made it all the way through our peer-review and QA process and ultimatly landed on our live server. Luckily for us, it was a pretty easy fix and we had a hotfix deployed within a matter of minutes after discovering the oversight. Crisis averted. But it got me thinking how horribly wrong this could have gone.
You see, I was the one that wrote this "asdf fdsa" string while trying to do some debugging on a view with some particularly complicated output rewriting. It's something I do freqently, both in code and in things like views. It helps me visualize where in the page rendering flow things are breaking down. I do this a lot too in my module code just to get a sense of what's breaking and where:
dsm('testing things');
Except, I don't always write "testing things". Sometimes I write "tacos". Sometimes I write "you are here". Sometimes I fight with views for 4 hours on what I thought was going to be a 30 minute task and, suffice it to say, I wear my emotions on my sleeve... There are things I could have written in this debug code that would turned this from a simple mistake into an absolute nightmare. Imagine if, in my frustration, I had written something like "UGH, this is broken. Whoever wrote this is an idiot." or worse! I'm sure I could find ways to cause all kinds of drama with some simple debug code.
So this was ultimitely a nice reminder. It's okay to be frustrated. It's okay to stomp your feet or make faces at your computer. Sometimes problems are just really tricky and we need to blow off steam. Just do yourself a favor and don't do it in code. Because no matter how good your process, things can still get through!
Comments
mmm, debug tacos.
Great article -- it's good to see that others make the same mistakes from time to time. However, you referred to your mistake as an "oversite" numerous times. The correct word is "oversight." I found it ironic. :) Amazing service you have, thank you for everything you do.
The best part about the debug code is that once you work with a team long enough you can pretty much tell who added which pieces of debug output. My goto phrase is "beep".
It would be super if this article were followed up with resources or more information to teach people how to accomplish what you were trying to debug with a real debugger.
Add new comment