4 ms·

I care more about full functional test coverage than code review. The tests must be automated, regression and functional coverage not just unit test. Test r

I care more about full functional test coverage than code review. The tests must be automated, regression and functional coverage not just unit test. Test results/logs must output in HTML store in DB backend (now) or excel (did that in 1997) for ease of analysis. Test failures are highlight in red, passes are in green. The developers should write new test code for every new feature and run all the existing test code after/merge before checkin. All the projects I worked on that implement this process are very successful. With this process in place and agree upon, I careless about code review. With enough "functional" test coverage, it is easy to do massive refactoring of code without worry about any breaking. Worked on one project that try code review for two weeks - other than a few comments about coding style, not much gain from the time spent.

zwieback 13y ago
Tests==code. If your test code is not reviewed then you don't know if your tests are any good. Neither code reviews nor TDD are silver bullets.
DougWebb 13y ago
Test coverage is pretty orthogonal to code review. You might wind up with a large amount of fully tested freshly refactored code that does not meet requirements or edge cases the tests missed. I'd recommend at least reviewing the test code in your case. Besides that, it sounds like a nice codebase to work in. I've had very good test coverage for libraries and backend services with well-defined APIs I've written, but a lot of my career has been spent doing UI work and I've never found an adequate testing tool that can deal with both the complexity and randomness of human interaction and the rate of change of UI presentation. You can't really regression test UIs, because most UI development intentionally changes the things regression tests look for, so you wind up spending a lot of time updating the regression tests and trying to keep up with UI changes that are being made in a tight code/review/tweak iteration loop.
RougeFemme 13y ago
Code review, done properly, should provide more than just a critique of coding style. It should should also catch certain types of bugs before they get to functional testing. Yes, you can catch those same bugs in functional tests, but in the environments where I've worked, there's a lot more overhead (read: time, money) involved with bugs found in functional test than in code review. Ita can be pretty frustrating to be that far along (functional test) and find a bug that could've easily been found in code review.

zwieback 13y ago

Tests==code. If your test code is not reviewed then you don't know if your tests are any good. Neither code reviews nor TDD are silver bullets.

DougWebb 13y ago

Test coverage is pretty orthogonal to code review. You might wind up with a large amount of fully tested freshly refactored code that does not meet requirements or edge cases the tests missed. I'd recommend at least reviewing the test code in your case. Besides that, it sounds like a nice codebase to work in. I've had very good test coverage for libraries and backend services with well-defined APIs I've written, but a lot of my career has been spent doing UI work and I've never found an adequate testing tool that can deal with both the complexity and randomness of human interaction and the rate of change of UI presentation. You can't really regression test UIs, because most UI development intentionally changes the things regression tests look for, so you wind up spending a lot of time updating the regression tests and trying to keep up with UI changes that are being made in a tight code/review/tweak iteration loop.

RougeFemme 13y ago

Code review, done properly, should provide more than just a critique of coding style. It should should also catch certain types of bugs before they get to functional testing. Yes, you can catch those same bugs in functional tests, but in the environments where I've worked, there's a lot more overhead (read: time, money) involved with bugs found in functional test than in code review. Ita can be pretty frustrating to be that far along (functional test) and find a bug that could've easily been found in code review.