Share via

Code reviews on the test team

  • Article

I've mentioned a few times in the past that before we (the test team) check in any code we have to get it reviewed. I don't think I ever mentioned the process before so here is what we do. This will pretty much follow industry standard guidelines and practices so there won't be anything outlandish or groundbreaking here, but I did want to give everyone an idea of our expectations.

First, check out and make the code changes you need.

Next, we package the changes into a .DPK file. We have command line utilities to do this, or mostly we use the code review tool - an in-house created tool - to grab the changes and upload them to a common server. This same tool also provides a windiff type of display so we can see the changes. This tool also allows comments to be made so we can write up notes on the code. Oddly, we don’t use OneNote for this…

Anyway, an email will go out to the entire test team that there are changes to be reviewed. Or, you can send the mail to specific stakeholders if that is appropriate. Anyone that wants to can jump in and review the changes, and although we have a requirement that at least one other person reviews the changes, there is no requirement that any one person has to review this particular change. This may vary a bit - for instance, the key ink testers review ink automation, the storage folks look at those tests, and so on. Here we check for design, implementation, comments, clarity and coding guidelines - again, pretty standard items. If we are satisfied, we can mark the review complete. If there are non-trivial changes, the author has to make the changes (or defend her code and implementation) and either submit a new review for another round or get agreement from the dissenting tester that the code is sufficient as is.

The final key here is that you have to get buyoff from everyone that participated before you can check in. Even if I say that the changes are sufficient, if Alice does not agree, you have to work with Alice to reach agreement.

Then you can check in your changes and move on. In general, this should not take more than a day or so to finish end to end.

Questions, comments, concerns and criticisms always welcome,

John

Comments

  • Anonymous
    July 09, 2013
    Looks like a fine process.  I guess my only addition might be that I'd want to be able to check my code into the code repository as "In-Test", then when the reviewer approves it, the status changes to "Production," or something like that.  We like to have code changes put into the repository even if they don't end up in production, if for no other reason than we have a copy that's not on the developer's laptop, susceptible to hardware loss, vacation, etc. Thanks for sharing.

  • Anonymous
    July 09, 2013
    The tool we have uploads a copy of the changes to a server that is meant for simply making it available to be reviewed, but it also doubles as a backup in case something goes horribly bad with a local hard drive.  Good point though - makes me think part of this could be automated a bit better...