Status: Available
Owner: ----
CC: [email protected]
Labels: Type-Feature Pri-2 OS-All Area-DevTools

New issue 6056 by [email protected]: Enforce unit testing in gcl
http://code.google.com/p/chromium/issues/detail?id=6056

As a way to help contributors and reviewers remember to add unit 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
   o 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.*
   o 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
   o 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.

--
You received this message because you are listed in the owner
or CC fields of this issue, or because you starred this issue.
You may adjust your issue notification preferences at:
http://code.google.com/hosting/settings

--~--~---------~--~----~------------~-------~--~----~
Automated mail from issue updates at http://crbug.com/
Subscription options: http://groups.google.com/group/chromium-bugs
-~----------~----~----~----~------~----~------~--~---

Reply via email to