More PRESUBMIT checks.
Checks for: - No iostream includes in headers - No use of FRIEND_TEST for gtest - Verifies that all C/C++ code passes cpplint.py check. - Verifies that BUG= is present in commit message - Verifies that TEST= is present in commit message For more details, see Chrome's PRESUBMIT.py at http://src.chromium.org/viewvc/chrome/trunk/src/PRESUBMIT.py?revision=113979&view=markup and the canned checks at http://src.chromium.org/viewvc/chrome/trunk/tools/depot_tools/presubmit_canned_checks.py?view=markup BUG= TEST= Review URL: https://webrtc-codereview.appspot.com/317011 git-svn-id: http://webrtc.googlecode.com/svn/trunk@1737 4adac7df-926f-26a2-2b94-8c16560cd09d
This commit is contained in:
90
PRESUBMIT.py
90
PRESUBMIT.py
@ -23,8 +23,91 @@ def _LicenseHeader(input_api):
|
|||||||
}
|
}
|
||||||
return license_header
|
return license_header
|
||||||
|
|
||||||
|
def _CheckNoIOStreamInHeaders(input_api, output_api):
|
||||||
|
"""Checks to make sure no .h files include <iostream>."""
|
||||||
|
files = []
|
||||||
|
pattern = input_api.re.compile(r'^#include\s*<iostream>',
|
||||||
|
input_api.re.MULTILINE)
|
||||||
|
for f in input_api.AffectedSourceFiles(input_api.FilterSourceFile):
|
||||||
|
if not f.LocalPath().endswith('.h'):
|
||||||
|
continue
|
||||||
|
contents = input_api.ReadFile(f)
|
||||||
|
if pattern.search(contents):
|
||||||
|
files.append(f)
|
||||||
|
|
||||||
|
if len(files):
|
||||||
|
return [ output_api.PresubmitError(
|
||||||
|
'Do not #include <iostream> in header files, since it inserts static ' +
|
||||||
|
'initialization into every file including the header. Instead, ' +
|
||||||
|
'#include <ostream>. See http://crbug.com/94794',
|
||||||
|
files) ]
|
||||||
|
return []
|
||||||
|
|
||||||
|
def _CheckNoFRIEND_TEST(input_api, output_api):
|
||||||
|
"""Make sure that gtest's FRIEND_TEST() macro is not used, the
|
||||||
|
FRIEND_TEST_ALL_PREFIXES() macro from testsupport/gtest_prod_util.h should be
|
||||||
|
used instead since that allows for FLAKY_, FAILS_ and DISABLED_ prefixes."""
|
||||||
|
problems = []
|
||||||
|
|
||||||
|
file_filter = lambda f: f.LocalPath().endswith(('.cc', '.h'))
|
||||||
|
for f in input_api.AffectedFiles(file_filter=file_filter):
|
||||||
|
for line_num, line in f.ChangedContents():
|
||||||
|
if 'FRIEND_TEST(' in line:
|
||||||
|
problems.append(' %s:%d' % (f.LocalPath(), line_num))
|
||||||
|
|
||||||
|
if not problems:
|
||||||
|
return []
|
||||||
|
return [output_api.PresubmitPromptWarning('WebRTC\'s code should not use '
|
||||||
|
'gtest\'s FRIEND_TEST() macro. Include testsupport/gtest_prod_util.h and '
|
||||||
|
'use FRIEND_TEST_ALL_PREFIXES() instead.\n' + '\n'.join(problems))]
|
||||||
|
|
||||||
|
def _CheckNewFilesLintClean(input_api, output_api, source_file_filter=None):
|
||||||
|
"""Checks that all NEW '.cc' and '.h' files pass cpplint.py.
|
||||||
|
This check is based on _CheckChangeLintsClean in
|
||||||
|
depot_tools/presubmit_canned_checks.py but has less filters and only checks
|
||||||
|
added files."""
|
||||||
|
result = []
|
||||||
|
|
||||||
|
# Initialize cpplint.
|
||||||
|
import cpplint
|
||||||
|
# Access to a protected member _XX of a client class
|
||||||
|
# pylint: disable=W0212
|
||||||
|
cpplint._cpplint_state.ResetErrorCounts()
|
||||||
|
|
||||||
|
# Justifications for each filter:
|
||||||
|
#
|
||||||
|
# - build/header_guard : WebRTC coding style says they should be prefixed
|
||||||
|
# with WEBRTC_, which is not possible to configure in
|
||||||
|
# cpplint.py.
|
||||||
|
cpplint._SetFilters('-build/header_guard')
|
||||||
|
|
||||||
|
# Use the strictest verbosity level for cpplint.py (level 1) which is the
|
||||||
|
# default when running cpplint.py from command line.
|
||||||
|
# To make it possible to work with not-yet-converted code, we're only applying
|
||||||
|
# it to new (or moved/renamed) files.
|
||||||
|
verbosity_level = 1
|
||||||
|
files = []
|
||||||
|
for f in input_api.AffectedSourceFiles(source_file_filter):
|
||||||
|
# Note that moved/renamed files also count as added for svn.
|
||||||
|
if (f.Action() == 'A'):
|
||||||
|
files.append(f.AbsoluteLocalPath())
|
||||||
|
for file_name in files:
|
||||||
|
cpplint.ProcessFile(file_name, verbosity_level)
|
||||||
|
|
||||||
|
if cpplint._cpplint_state.error_count > 0:
|
||||||
|
if input_api.is_committing:
|
||||||
|
# TODO(kjellander): Change back to PresubmitError below when we're
|
||||||
|
# confident with the lint settings.
|
||||||
|
res_type = output_api.PresubmitPromptWarning
|
||||||
|
else:
|
||||||
|
res_type = output_api.PresubmitPromptWarning
|
||||||
|
result = [res_type('Changelist failed cpplint.py check.')]
|
||||||
|
|
||||||
|
return result
|
||||||
|
|
||||||
def _CommonChecks(input_api, output_api):
|
def _CommonChecks(input_api, output_api):
|
||||||
"""Checks common to both upload and commit."""
|
"""Checks common to both upload and commit."""
|
||||||
|
# TODO(kjellander): Use presubmit_canned_checks.PanProjectChecks too.
|
||||||
results = []
|
results = []
|
||||||
results.extend(input_api.canned_checks.CheckLongLines(
|
results.extend(input_api.canned_checks.CheckLongLines(
|
||||||
input_api, output_api))
|
input_api, output_api))
|
||||||
@ -34,8 +117,11 @@ def _CommonChecks(input_api, output_api):
|
|||||||
input_api, output_api))
|
input_api, output_api))
|
||||||
results.extend(input_api.canned_checks.CheckChangeTodoHasOwner(
|
results.extend(input_api.canned_checks.CheckChangeTodoHasOwner(
|
||||||
input_api, output_api))
|
input_api, output_api))
|
||||||
|
results.extend(_CheckNewFilesLintClean(input_api, output_api))
|
||||||
results.extend(input_api.canned_checks.CheckLicense(
|
results.extend(input_api.canned_checks.CheckLicense(
|
||||||
input_api, output_api, _LicenseHeader(input_api)))
|
input_api, output_api, _LicenseHeader(input_api)))
|
||||||
|
results.extend(_CheckNoIOStreamInHeaders(input_api, output_api))
|
||||||
|
results.extend(_CheckNoFRIEND_TEST(input_api, output_api))
|
||||||
return results
|
return results
|
||||||
|
|
||||||
def CheckChangeOnUpload(input_api, output_api):
|
def CheckChangeOnUpload(input_api, output_api):
|
||||||
@ -51,4 +137,8 @@ def CheckChangeOnCommit(input_api, output_api):
|
|||||||
input_api, output_api))
|
input_api, output_api))
|
||||||
results.extend(input_api.canned_checks.CheckChangeHasDescription(
|
results.extend(input_api.canned_checks.CheckChangeHasDescription(
|
||||||
input_api, output_api))
|
input_api, output_api))
|
||||||
|
results.extend(input_api.canned_checks.CheckChangeHasBugField(
|
||||||
|
input_api, output_api))
|
||||||
|
results.extend(input_api.canned_checks.CheckChangeHasTestField(
|
||||||
|
input_api, output_api))
|
||||||
return results
|
return results
|
||||||
|
|||||||
@ -41,6 +41,7 @@
|
|||||||
'testsupport/frame_reader.cc',
|
'testsupport/frame_reader.cc',
|
||||||
'testsupport/frame_writer.h',
|
'testsupport/frame_writer.h',
|
||||||
'testsupport/frame_writer.cc',
|
'testsupport/frame_writer.cc',
|
||||||
|
'testsupport/gtest_prod_util.h',
|
||||||
'testsupport/packet_reader.h',
|
'testsupport/packet_reader.h',
|
||||||
'testsupport/packet_reader.cc',
|
'testsupport/packet_reader.cc',
|
||||||
'testsupport/mock/mock_frame_reader.h',
|
'testsupport/mock/mock_frame_reader.h',
|
||||||
|
|||||||
36
test/testsupport/gtest_prod_util.h
Normal file
36
test/testsupport/gtest_prod_util.h
Normal file
@ -0,0 +1,36 @@
|
|||||||
|
/*
|
||||||
|
* Copyright (c) 2012 The WebRTC project authors. All Rights Reserved.
|
||||||
|
*
|
||||||
|
* Use of this source code is governed by a BSD-style license
|
||||||
|
* that can be found in the LICENSE file in the root of the source
|
||||||
|
* tree. An additional intellectual property rights grant can be found
|
||||||
|
* in the file PATENTS. All contributing project authors may
|
||||||
|
* be found in the AUTHORS file in the root of the source tree.
|
||||||
|
*/
|
||||||
|
|
||||||
|
#ifndef WEBRTC_TEST_TESTSUPPORT_GTEST_PROD_UTIL_H_
|
||||||
|
#define WEBRTC_TEST_TESTSUPPORT_GTEST_PROD_UTIL_H_
|
||||||
|
#pragma once
|
||||||
|
|
||||||
|
#include "gtest/gtest_prod.h"
|
||||||
|
|
||||||
|
// This file is a plain copy of Chromium's base/gtest_prod_util.h.
|
||||||
|
//
|
||||||
|
// This is a wrapper for gtest's FRIEND_TEST macro that friends
|
||||||
|
// test with all possible prefixes. This is very helpful when changing the test
|
||||||
|
// prefix, because the friend declarations don't need to be updated.
|
||||||
|
//
|
||||||
|
// Example usage:
|
||||||
|
//
|
||||||
|
// class MyClass {
|
||||||
|
// private:
|
||||||
|
// void MyMethod();
|
||||||
|
// FRIEND_TEST_ALL_PREFIXES(MyClassTest, MyMethod);
|
||||||
|
// };
|
||||||
|
#define FRIEND_TEST_ALL_PREFIXES(test_case_name, test_name) \
|
||||||
|
FRIEND_TEST(test_case_name, test_name); \
|
||||||
|
FRIEND_TEST(test_case_name, DISABLED_##test_name); \
|
||||||
|
FRIEND_TEST(test_case_name, FLAKY_##test_name); \
|
||||||
|
FRIEND_TEST(test_case_name, FAILS_##test_name)
|
||||||
|
|
||||||
|
#endif // WEBRTC_TEST_TESTSUPPORT_GTEST_PROD_UTIL_H_
|
||||||
Reference in New Issue
Block a user