11

I am debating with my colleague on when to perform code review - before or after unit tests. What is the best practice?

Some factors we might need to take into account (there may be more):

  • Size of code change - a big change means more changes will result from the code review. If these changes are big than, if UT was before code review, you'll need to repeat most of your UTs again.
  • Time required to perform unit test
  • Is it new functionality or a bug fix
Improve this question
1
  • I personally don't think the two are so dependant on one another. Developers should only review complete code, because it may be incomplete or not working as expected. Commented Nov 23, 2011 at 10:03

5 Answers 5

Reset to default
21

You should always unit test before doing the code review and here's why

  1. If your code is broken in a way that would be caught by unit tests you will be wasting the other developer's time by getting them involved in the red/green/refactor cycle.
  2. Tests show other developers the intended use of the code which makes it easier to review.
  3. Tests should be reviewed along with the code that is tested in case you are missing test cases or your tests don't work properly.
  4. Tests and code review tend to catch different issues with only a little overlap in issues found. Unit tests don't get annoyed at having to retest code when the reviewer finds issues, developers do get annoyed and probably won't do as well the second time through.

There are probably other reasons but those are the ones I've personally seen and experienced having implemented code review practices within 3 different teams/companies.

Edit Of course the above is for times when code review is a step in your software development process (whether waterfall or agile). If you're working on a particularly large or difficult section of code feel free to get another pair of eyes on it at any point.

Improve this answer
11

Code Reviews are for when the code is "done".

In my organisation our definition of "done" includes unit tests (as we aim for TDD) so code reviews are of complete code - and complete code includes tests.

Also, tests need reviewing and refactoring so it make sense that they are part of the code review.

Improve this answer
4
  • Don't it make sense to make code review to code before you write unit tests for it? Commented Nov 23, 2011 at 10:08
  • if you have tests and the code review suggests changes to the code, you can you make the changes to the code with confidence as they are supported by tests. Without tests, changes resulting from the code review might introduce bugs. Commented Nov 23, 2011 at 10:11
  • Ok, maybe I didn't explained myself good. What I mean is a case when your code is for completely new functionality and yet not covered by unit tests. Will it be good to perform code review to code, before you write unit tests for this new functionally? Commented Nov 23, 2011 at 10:15
  • Hi Dimba. I'm not sure there is an absolute best way to be honest. Personally I would have the code review after the tests were written, but that's because I know myself and the preferences of the people I work with. Perhaps try each technique and see which you / your team prefer? The main thing is you have tests - so well done there. Commented Nov 23, 2011 at 11:38
4

Tests should be considered part of the code to review. Therefore it makes sense to review after tests are done.

Ensure the tests are reviewed as well. This is critical for those who are new to unit tests.

Make sure your team underetsnds dependency injection, isolation frameworks, mocks vs stubs, seams, interaction vs state based tests, and integration vs unit tests.

You don't need to implement the aforementioned topics, but you should understand them.

Improve this answer
2

Well,

This depends on what you mean by "Unit Test"...

If it was a TDD style Unit Test it is meaningless because you write test while you write your code. There is no after-later case.In this case you improve code quality continously: Refactoring...

AND

If it was classic "unit test" [ whatever it means i do not know, but i mean test after you write the codes and done by usually other guys] then the main criteria is what you expect from codereview and nature of unit tests: if you want fast feedback-make review and take action and have no automated unit test,you will have to wait unit test. If you want to identify mature problems with code review, and incrementally apply solution for next iterations, you may do it before unit test...

But after all personally, for codereview, after or later unit test is not a real criteria for me...

Why we do codereview? For code quality...Instead of a "quality control" gate, inject quality into your software developement process-enviroment...

Improve this answer
1
  • @Thanks for answer. Maybe I wasn't clear, but I don't refer code review as some sort of formal "quality control" gate. I'm trying to see what is "correct" way in terms of speed/quality of development Commented Nov 23, 2011 at 14:00
2

I'd tend to say, let's be "agile"... don't wait for the code to be finished to make some quick, informal code review: there are developers with whom and subjects with which you can indeed wait for the entire code + test phasis to be finished... but

when it comes to really new subjects (whole new feature, near-research, something totally new to the team), code review early, don't loose time: have a coworker get a look from time to time: isolation is an important factor of failure in this case.

if the developer is new to the team, as well, review code early and maybe often.

and by the way, unit tests as well need code review.

Improve this answer

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.