We don't have very good unit test coverage (in the broad sense, including
ui_tests, test_shell_tests, etc.) for our code. We've always had a policy
that any new code had to have an associated test, but historically we've
been really bad about enforcing it.
As a way to help contributors and reviewers remember to add tests, here's a
proposal to have gcl report when they are (or might be) missing. It's a
very rough guess now, and will probably need some refinement as we see what
it misses and where its false positives are too annoying.
What changes might need tests?
- Any new source file (.cc, .cpp, .m, or .mm), or
- Any new method added to a source or header (.h file
- A new method is identified by a flush-left non-comment line that has
( somewhere before the next flush-left line and either ends with
{ or has {
at the start of the next flush-left line.
What counts as a test?
- Any change to any code file whose name ends in test.* or tests.*
- This is very rough, but at least it shows that the contributor
thought about testing when making the patch
What do we do if we don't find a test?
- On 'gcl change', report a warning to the user
- On 'gcl upload', add a warning to the change description so the
reviewer sees it too
- Add an option to override this
Future possibilities
- Is it worth restricting the check to only public or protected methods?
- Since any "real" change ought to either fix a bug or add a feature that
should be tested, warn whenever there are no changes to any tests (including
layout tests) or a test_lists file, even when no source files or methods
were added. Alas, this is probably not feasible since we don't keep layout
tests in the same repository
- Rather than adding a warning to the change description, it'd be nice to
have a separate warning in the review app, so it showed up no matter what
and the path author didn't have to override anything. But since we want the
warning in client-side 'gcl change' anyway, for now we'll keep it simple and
trust people.
Comments and volunteers welcome.
- Pam
--~--~---------~--~----~------------~-------~--~----~
Chromium Developers mailing list: [email protected]
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
-~----------~----~----~----~------~----~------~--~---
