From e8bfedab96e81247b2b426d09e221b52e05c6af9 Mon Sep 17 00:00:00 2001 From: machenbach Date: Wed, 25 Nov 2015 05:18:49 -0800 Subject: [PATCH] [test] Add status-file presubmit check. This loads all test suites and status files to catch subtle syntax errors. It also checks basic status file integrity and common mistakes. NOTRY=true Review URL: https://codereview.chromium.org/1475663002 Cr-Commit-Position: refs/heads/master@{#32271} --- PRESUBMIT.py | 3 +++ tools/presubmit.py | 16 ++++++++++++++ tools/testrunner/local/statusfile.py | 31 ++++++++++++++++++++++++++-- 3 files changed, 48 insertions(+), 2 deletions(-) diff --git a/PRESUBMIT.py b/PRESUBMIT.py index ab9bba8845..dd7c32f07e 100644 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py @@ -69,6 +69,7 @@ def _V8PresubmitChecks(input_api, output_api): from presubmit import SourceProcessor from presubmit import CheckExternalReferenceRegistration from presubmit import CheckAuthorizedAuthor + from presubmit import CheckStatusFiles results = [] if not CppLintProcessor().Run(input_api.PresubmitLocalPath()): @@ -80,6 +81,8 @@ def _V8PresubmitChecks(input_api, output_api): if not CheckExternalReferenceRegistration(input_api.PresubmitLocalPath()): results.append(output_api.PresubmitError( "External references registration check failed")) + if not CheckStatusFiles(input_api.PresubmitLocalPath()): + results.append(output_api.PresubmitError("Status file check failed")) results.extend(CheckAuthorizedAuthor(input_api, output_api)) return results diff --git a/tools/presubmit.py b/tools/presubmit.py index bd1804712b..0e86063d6d 100755 --- a/tools/presubmit.py +++ b/tools/presubmit.py @@ -45,6 +45,10 @@ import subprocess import multiprocessing from subprocess import PIPE +from testrunner.local import statusfile +from testrunner.local import testsuite +from testrunner.local import utils + # Special LINT rules diverging from default and reason. # build/header_guard: Our guards have the form "V8_FOO_H_", not "SRC_FOO_H_". # build/include_what_you_use: Started giving false positives for variables @@ -403,6 +407,17 @@ def CheckExternalReferenceRegistration(workspace): [sys.executable, join(workspace, "tools", "external-reference-check.py")]) return code == 0 +def CheckStatusFiles(workspace): + suite_paths = utils.GetSuitePaths(join(workspace, "test")) + for root in suite_paths: + suite_path = join(workspace, "test", root) + status_file_path = join(suite_path, root + ".status") + suite = testsuite.TestSuite.LoadTestSuite(suite_path) + if suite and exists(status_file_path): + if not statusfile.PresubmitCheck(status_file_path): + return False + return True + def CheckAuthorizedAuthor(input_api, output_api): """For non-googler/chromites committers, verify the author's email address is in AUTHORS. @@ -450,6 +465,7 @@ def Main(): "two empty lines between declarations check..." success = SourceProcessor().Run(workspace) and success success = CheckExternalReferenceRegistration(workspace) and success + success = CheckStatusFiles(workspace) and success if success: return 0 else: diff --git a/tools/testrunner/local/statusfile.py b/tools/testrunner/local/statusfile.py index bfa53c5348..6e2a012a9a 100644 --- a/tools/testrunner/local/statusfile.py +++ b/tools/testrunner/local/statusfile.py @@ -25,6 +25,7 @@ # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +import os # These outcomes can occur in a TestCase's outcomes list: SKIP = "SKIP" @@ -125,10 +126,14 @@ def _ParseOutcomeList(rule, outcomes, target_dict, variables): target_dict[rule] = result -def ReadStatusFile(path, variables): +def ReadContent(path): with open(path) as f: global KEYWORDS - contents = eval(f.read(), KEYWORDS) + return eval(f.read(), KEYWORDS) + + +def ReadStatusFile(path, variables): + contents = ReadContent(path) rules = {} wildcards = {} @@ -146,3 +151,25 @@ def ReadStatusFile(path, variables): else: _ParseOutcomeList(rule, section[rule], rules, variables) return rules, wildcards + + +def PresubmitCheck(path): + contents = ReadContent(path) + root_prefix = os.path.basename(os.path.dirname(path)) + "/" + + try: + for section in contents: + assert type(section) == list + assert len(section) == 2 + section = section[1] + assert type(section) == dict + for rule in section: + assert type(rule) == str + assert not rule.startswith(root_prefix), ( + "Suite name prefix must not be used in status files") + assert not rule.endswith('.js'), ( + ".js extension must not be used in status files.") + return True + except Exception as e: + print e + return False