pre-upload: fix json lint to read files from the commit
We shouldn't read files off the current disk, but get all the content
out of git. This helps with stacked changes and dirty trees.
BUG=None
TEST=unittests pass
Change-Id: I462481aac7711b6994c2aca99253aa0b738157e0
Reviewed-on: https://chromium-review.googlesource.com/849137
Commit-Ready: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>
diff --git a/pre-upload.py b/pre-upload.py
index 375f695..4c6b783 100755
--- a/pre-upload.py
+++ b/pre-upload.py
@@ -1255,11 +1255,19 @@
def _run_json_check(_project, commit):
"""Checks that all JSON files are syntactically valid."""
- for f in _filter_files(_get_affected_files(commit), [r'.*\.json']):
+ ret = []
+
+ files = _filter_files(_get_affected_files(commit, relative=True),
+ [r'.*\.json$'])
+ for f in files:
+ data = _get_file_content(f, commit)
try:
- json.load(open(f))
- except Exception, e:
- return HookFailure('Invalid JSON in %s: %s' % (f, e))
+ json.loads(data)
+ except Exception as e:
+ ret.append('%s: Invalid JSON: %s' % (f, e))
+
+ if ret:
+ return HookFailure('\n'.join(ret))
def _check_manifests(_project, commit):
diff --git a/pre-upload_unittest.py b/pre-upload_unittest.py
index e38fb96..0f36910 100755
--- a/pre-upload_unittest.py
+++ b/pre-upload_unittest.py
@@ -222,6 +222,45 @@
self.assertFalse(failure)
+class CheckJson(cros_test_lib.MockTestCase):
+ """Tests for _run_json_check."""
+
+ def setUp(self):
+ self.file_mock = self.PatchObject(pre_upload, '_get_affected_files')
+ self.content_mock = self.PatchObject(pre_upload, '_get_file_content')
+
+ def testNoJson(self):
+ """Nothing should be checked w/no JSON files."""
+ self.file_mock.return_value = [
+ '/foo/bar.txt',
+ '/readme.md',
+ ]
+ ret = pre_upload._run_json_check('PROJECT', 'COMMIT')
+ self.assertIsNone(ret)
+
+ def testValidJson(self):
+ """We should accept valid json files."""
+ self.file_mock.return_value = [
+ '/foo/bar.txt',
+ '/data.json',
+ ]
+ self.content_mock.return_value = '{}'
+ ret = pre_upload._run_json_check('PROJECT', 'COMMIT')
+ self.assertIsNone(ret)
+ self.content_mock.assert_called_once_with('/data.json', 'COMMIT')
+
+ def testInvalidJson(self):
+ """We should reject invalid json files."""
+ self.file_mock.return_value = [
+ '/foo/bar.txt',
+ '/data.json',
+ ]
+ self.content_mock.return_value = '{'
+ ret = pre_upload._run_json_check('PROJECT', 'COMMIT')
+ self.assertIsNotNone(ret)
+ self.content_mock.assert_called_once_with('/data.json', 'COMMIT')
+
+
class CheckManifests(cros_test_lib.MockTestCase):
"""Tests _check_manifests."""
