From fbb06cdb231ecf07f330be6d30a8777cd1e40fd5 Mon Sep 17 00:00:00 2001 From: Yiwei Zhang Date: Mon, 12 Jun 2023 16:17:23 +0000 Subject: [PATCH] config: output warnings for validation error/warnings in non-affected files R=iannucci, sokcevic, yuanjunh Bug: 1449933 Change-Id: I863c55f5a12da103b4167c8cf9a1aac02962838f Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4602126 Reviewed-by: Yuanjun Huang Reviewed-by: Robbie Iannucci Commit-Queue: Yiwei Zhang Reviewed-by: Josip Sokcevic --- presubmit_canned_checks.py | 19 +++++++++++++++---- tests/presubmit_unittest.py | 7 ++++++- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py index c28e6e8f13..63b728747f 100644 --- a/presubmit_canned_checks.py +++ b/presubmit_canned_checks.py @@ -1918,6 +1918,7 @@ def CheckChangedLUCIConfigs(input_api, output_api): outputs = [] lucicfg = 'lucicfg' if not input_api.is_windows else 'lucicfg.bat' log_level = 'debug' if input_api.verbose else 'warning' + repo_root = input_api.change.RepositoryRoot() for d, fileSet in dir_to_fileSet.items(): config_set = dir_to_config_set[d] with input_api.CreateTemporaryFile() as f: @@ -1931,7 +1932,7 @@ def CheckChangedLUCIConfigs(input_api, output_api): cmd, stderr=input_api.subprocess.PIPE, shell=input_api.is_windows, # to resolve *.bat - cwd=input_api.change.RepositoryRoot(), + cwd=repo_root, ) logging.debug('running %s\nSTDOUT:\n%s\nSTDERR:\n%s', cmd, out[0], out[1]) try: @@ -1943,12 +1944,11 @@ def CheckChangedLUCIConfigs(input_api, output_api): else: result = result.get('result', None) if result: + non_affected_file_msg_count = 0 for validation_result in (result.get('validation', None) or []): for msg in (validation_result.get('messages', None) or []): if d != '.' and msg['path'] not in fileSet: - # TODO(yiwzhang): This is the message from non-affected file. - # Output presubmit warning for those files if needed in the - # future. + non_affected_file_msg_count += 1 continue sev = msg['severity'] if sev == 'WARNING': @@ -1960,6 +1960,17 @@ def CheckChangedLUCIConfigs(input_api, output_api): outputs.append( out_f('Config validation for file(%s): %s' % (msg['path'], msg['text']))) + if non_affected_file_msg_count: + reproduce_cmd = [ + lucicfg, 'validate', + repo_root if d == '.' else input_api.os_path.join(repo_root, d), + '-config-set', config_set + ] + outputs.append( + output_api.PresubmitPromptWarning( + 'Found %d additional errors/warnings in files that are not ' + 'modified, run `%s` to reveal them' % + (non_affected_file_msg_count, ' '.join(reproduce_cmd)))) return outputs diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index f0fc6bb94f..6b7e6d33c4 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -2219,9 +2219,14 @@ class CannedChecksUnittest(PresubmitTestsBase): results = presubmit_canned_checks.CheckChangedLUCIConfigs( input_api, presubmit.OutputApi) - self.assertEqual(len(results), 1) + self.assertEqual(len(results), 2) self.assertEqual(results[0].json_format()['message'], "Config validation for file(bar.cfg): deadbeef") + self.assertEqual( + results[1].json_format()['message'], + "Found 1 additional errors/warnings in files that are not modified," + " run `lucicfg validate %s%sgenerated -config-set " + "project/deadbeef` to reveal them" % (self.fake_root_dir, self._OS_SEP)) subprocess.Popen.assert_called_once_with([ 'lucicfg' + ('.bat' if input_api.is_windows else ''), 'validate',