Does anyone practice the "code review" process for functional tests? Do you find it useful? The way that my current employer practices SCRUM we include functional testing as part of our "must have done" stuff in any given sprint.
We also practice SCRUM. And the same as you, we also include functional testing as part of our definition as done.
From my experience, I find it incredibly useful. We have significantly reduced the number of bugs in our code by simply forcing the functional testing.
A second nice thing about the code review, is it gives you another view over the actual functionality, and to make 100% sure that it is in line with what the customer/client wanted. There has been a few times when someone was going over code and functionality where the person went ... "Hang on, this is not right..." and it turned out that the person implementing the code just misunderstood something.
Good heavens yes (I try not to use expletives on SO ;p). Peer reviewing your functional tests is basically peer reviewing your requirements and analysis, it's incredibly important, and if you use a BDD language like cucumber you can involve non programmers as well!
It's awesome when our end users spot issues with our functional tests and it makes them feel hugely part of the development process "I can read code too!!"
It makes perfect sense to me. Any code you write should be looked at by someone else, even if the code is only used internally and will never be run by the customer.
With the methodologies that put so much weight in testing the review of the tests becomes much more important, possibly required, sometimes more important that the review of the code itself since it is often assumed it can be replaced with any code that meets the same automated testing outcome.
Reviewing that the tests are correct is one aspect, that they are complete enough and accurate/representative is also very important.
Missing this point is one of the things that makes these methodologies look sloppy to outside reviewers.
You can do pair inspections!
Pair Inspections are:
A review of documents actively & informally as part of the authoring & document production cycle.
The reasons why this works well with testing is that:
- You can frequently inspect requirements or documents with more than one pair of eyes.
- You can involve more than just developers: Try BA with Test Lead, BA with a PM, BA with Dev.
- You can set re-curing meetings as part of an Agile process – Make sure to be serious about it with solid commitments from team members.
- You can use these pair inspections as part of a rapport-building exercise and communication exercise with your stakeholders. Get the conversation going!
We peer-review functional tests at least casually, and it is strongly encouraged at our organization to get everything code-reviewed.
I would recommend choosing your reviewer based off of your goals for the review. Coded tests might be best reviewed by both a dev (for code quality primarily) and another tester (for test coverage primarily). Tests with no code (using a harness, e.g., data driven tests) might be best reviewed only by another tester. Peer reviews are also a great way to encourage testers to learn from each other.