Skip to content

Commit

Permalink
[Extern Generation] Add a presubmit script to check externs not being…
Browse files Browse the repository at this point in the history
… updated

Add a presubmit script that checks for when extension api files are touched, but
the corresponding extern file is not. Right now, this is very simple - it only
checks that the extern is modified in some way (doesn't validate that it's the
*right* way), and is only a warning (because sometimes api file changes don't
cause extern changes). As an improvement, we would validate that the extern
files contain the proper content - let's do that later.

This will also be rolled out piecemeal, since many APIs don't currently have a
dedicated extern file. This change only imposes the check on bluetooth (because
it was handy) - if all goes well, we'll roll this out to all api files.

Also establish a dedicated extern folder, since it's silly for chrome-generated
externs to live in third_party/.

BUG=469920

Review URL: https://codereview.chromium.org/1718243003

Cr-Commit-Position: refs/heads/master@{#378018}
  • Loading branch information
rdcronin authored and Commit bot committed Feb 26, 2016
1 parent 655e762 commit 9ab806c
Show file tree
Hide file tree
Showing 4 changed files with 135 additions and 0 deletions.
3 changes: 3 additions & 0 deletions PRESUBMIT_test_mocks.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,9 @@ def NewContents(self):
def LocalPath(self):
return self._local_path

def AbsoluteLocalPath(self):
return self._local_path

def rfind(self, p):
"""os.path.basename is called on MockFile so we need an rfind method."""
return self._local_path.rfind(p)
Expand Down
37 changes: 37 additions & 0 deletions extensions/common/api/PRESUBMIT.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
# Copyright 2016 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.

"""Chromium presubmit script for src/extensions/common.
See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts
for more details on the presubmit API built into depot_tools.
"""

import sys


def _CheckExterns(input_api, output_api):
original_sys_path = sys.path

try:
sys.path.append(input_api.PresubmitLocalPath())
from externs_checker import ExternsChecker
finally:
sys.path = original_sys_path

join = input_api.os_path.join
api_root = input_api.PresubmitLocalPath()
externs_root = join(api_root, '..', '..', '..', 'third_party',
'closure_compiler', 'externs')

api_pairs = {
join(api_root, 'bluetooth.idl'): join(externs_root, 'bluetooth.js'),
# TODO(rdevlin.cronin): Add more!
}

return ExternsChecker(input_api, output_api, api_pairs).RunChecks()


def CheckChangeOnUpload(input_api, output_api):
return _CheckExterns(input_api, output_api)
32 changes: 32 additions & 0 deletions extensions/common/api/externs_checker.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# Copyright 2016 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.

class ExternsChecker(object):
_UPDATE_MESSAGE = """To update the externs, run:
src/ $ python tools/json_schema_compiler/compiler.py\
%s --root=. --generator=externs > %s"""

def __init__(self, input_api, output_api, api_pairs):
self._input_api = input_api
self._output_api = output_api
self._api_pairs = api_pairs

def RunChecks(self):
bad_files = []
affected = [f.AbsoluteLocalPath() for f in self._input_api.AffectedFiles()]
for path in affected:
pair = self._api_pairs.get(path)
if pair != None and pair not in affected:
bad_files.append({'source': path, 'extern': pair})
results = []
if bad_files:
replacements = (('<source_file>', '<output_file>') if len(bad_files) > 1
else (bad_files[0]['source'], bad_files[0]['extern']))
long_text = self._UPDATE_MESSAGE % replacements
results.append(self._output_api.PresubmitPromptWarning(
str('Found updated extension api files without updated extern files. '
'Please update the extern files.'),
[f['source'] for f in bad_files],
long_text))
return results
63 changes: 63 additions & 0 deletions extensions/common/api/externs_checker_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
#!/usr/bin/env python
# Copyright 2016 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.

import os
import sys
import unittest

from externs_checker import ExternsChecker

sys.path.append(
os.path.join(os.path.dirname(os.path.abspath(__file__)), '..', '..', '..'))

from PRESUBMIT_test_mocks import MockInputApi, MockOutputApi, MockFile


class ExternsCheckerTest(unittest.TestCase):
API_PAIRS = {'a': '1', 'b': '2', 'c': '3'}

def _runChecks(self, files):
input_api = MockInputApi()
input_api.files = [MockFile(f, '') for f in files]
output_api = MockOutputApi()
checker = ExternsChecker(input_api, output_api, self.API_PAIRS)
return checker.RunChecks()

def testModifiedSourceWithoutModifiedExtern(self):
results = self._runChecks(['b', 'test', 'random'])
self.assertEquals(1, len(results))
self.assertEquals(1, len(results[0].items))
self.assertEquals('b', results[0].items[0])
self.assertEquals(
'To update the externs, run:\n'
' src/ $ python tools/json_schema_compiler/compiler.py b --root=. '
'--generator=externs > 2',
results[0].long_text)

def testModifiedSourceWithModifiedExtern(self):
results = self._runChecks(['b', '2', 'test', 'random'])
self.assertEquals(0, len(results))

def testModifiedMultipleSourcesWithNoModifiedExterns(self):
results = self._runChecks(['b', 'test', 'c', 'random'])
self.assertEquals(1, len(results))
self.assertEquals(2, len(results[0].items))
self.assertTrue('b' in results[0].items)
self.assertTrue('c' in results[0].items)
self.assertEquals(
'To update the externs, run:\n'
' src/ $ python tools/json_schema_compiler/compiler.py <source_file> '
'--root=. --generator=externs > <output_file>',
results[0].long_text)

def testModifiedMultipleSourcesWithOneModifiedExtern(self):
results = self._runChecks(['b', 'test', 'c', 'random', '2'])
self.assertEquals(1, len(results))
self.assertEquals(1, len(results[0].items))
self.assertEquals('c', results[0].items[0])


if __name__ == '__main__':
unittest.main()

0 comments on commit 9ab806c

Please sign in to comment.