trybot_commit_size_checker.py: Show resource_sizes.py diff inline
* Removes obsolete command-line flags.
* Moves resource_sizes.py diff as well as failing size checks inline
Bug: 914654
Change-Id: I444da444a2bbed4a154ce1d4a64bb99549e81e86
Reviewed-on: https://chromium-review.googlesource.com/c/1407375
Reviewed-by: Eric Stevenson <estevenson@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622156}diff --git a/tools/binary_size/diagnose_bloat.py b/tools/binary_size/diagnose_bloat.py
index c1807647..a0c3c37c 100755
--- a/tools/binary_size/diagnose_bloat.py
+++ b/tools/binary_size/diagnose_bloat.py
@@ -148,13 +148,13 @@
return self._ResultLines()
def Summary(self):
- header_lines = [
+ footer_lines = [
+ '',
'For an explanation of these metrics, see:',
('https://chromium.googlesource.com/chromium/src/+/master/docs/speed/'
- 'binary_size/metrics.md#Metrics-for-Android'),
- '']
- return header_lines + self._ResultLines(
- include_sections=ResourceSizesDiff._SUMMARY_SECTIONS)
+ 'binary_size/metrics.md#Metrics-for-Android')]
+ return self._ResultLines(
+ include_sections=ResourceSizesDiff._SUMMARY_SECTIONS) + footer_lines
def ProduceDiff(self, before_dir, after_dir):
before = self._LoadResults(before_dir)
diff --git a/tools/binary_size/trybot_commit_size_checker.py b/tools/binary_size/trybot_commit_size_checker.py
index f023a1b..ca44c68 100755
--- a/tools/binary_size/trybot_commit_size_checker.py
+++ b/tools/binary_size/trybot_commit_size_checker.py
@@ -34,9 +34,8 @@
'for an explanation of Normalized APK Size')
_FAILURE_GUIDANCE = """
-Please look at the symbol diffs from the "Show Resource Sizes Diff",
-"Show Supersize Diff", and "Dex Method Count", and "Supersize HTML Report" bot
-steps. Try and understand the growth and see if it can be mitigated.
+Please look at size breakdowns, try to understand the growth, and see if it can
+be mitigated.
There is guidance at:
@@ -78,7 +77,7 @@
return cmp(self.name, other.name)
-def _CreateAndWriteMethodCountDelta(symbols, output_path):
+def _CreateAndWriteMethodCountDelta(symbols):
dex_symbols = symbols.WhereInSection(models.SECTION_DEX_METHOD)
dex_added = dex_symbols.WhereDiffStatusIs(models.DIFF_STATUS_ADDED)
dex_removed = dex_symbols.WhereDiffStatusIs(models.DIFF_STATUS_REMOVED)
@@ -91,31 +90,21 @@
lines.append('Removed: {}'.format(dex_removed_count))
lines.extend(sorted(s.full_name for s in dex_removed))
- if output_path:
- with open(output_path, 'w') as f:
- f.writelines(l + '\n' for l in lines)
-
return lines, _SizeDelta('Dex Methods', 'methods',
_MAX_DEX_METHOD_COUNT_INCREASE, dex_net_added,
_DEX_DETAILS)
-def _CreateAndWriteResourceSizesDelta(apk_name, before_dir, after_dir,
- output_path):
+def _CreateAndWriteResourceSizesDelta(apk_name, before_dir, after_dir):
sizes_diff = diagnose_bloat.ResourceSizesDiff(apk_name)
sizes_diff.ProduceDiff(before_dir, after_dir)
- lines = sizes_diff.Summary()
- if output_path:
- with open(output_path, 'w') as f:
- f.writelines(l + '\n' for l in lines)
-
- return lines, _SizeDelta(
+ return sizes_diff.Summary(), _SizeDelta(
'Normalized APK Size', 'bytes', _MAX_NORMALIZED_INCREASE,
sizes_diff.summary_stat.value, _NORMALIZED_APK_SIZE_DETAILS)
-def _CreateAndWriteSupersizeDiff(apk_name, before_dir, after_dir, output_path):
+def _CreateAndWriteSupersizeDiff(apk_name, before_dir, after_dir):
before_size_path = os.path.join(before_dir, apk_name + '.size')
after_size_path = os.path.join(after_dir, apk_name + '.size')
before = archive.LoadAndPostProcessSizeInfo(before_size_path)
@@ -123,11 +112,6 @@
size_info_delta = diff.Diff(before, after, sort=True)
lines = list(describe.GenerateLines(size_info_delta))
-
- if output_path:
- with open(output_path, 'w') as f:
- f.writelines(l + '\n' for l in lines)
-
return lines, size_info_delta
@@ -157,35 +141,22 @@
required=True,
help='Directory containing APK for the new build.')
parser.add_argument(
- '--resource-sizes-diff-path',
- help='Output path for the resource_sizes.py diff.')
- parser.add_argument(
- '--supersize-diff-path', help='Output path for the Supersize diff.')
- parser.add_argument(
- '--dex-method-count-diff-path',
- help='Output path for the dex method count diff.')
- parser.add_argument(
- '--ndjson-path', help='Output path for the Supersize HTML report.')
- parser.add_argument(
'--results-path',
required=True,
help='Output path for the trybot result .json file.')
parser.add_argument(
- '--staging-dir', help='Directory to write summary files to.')
+ '--staging-dir',
+ required=True,
+ help='Directory to write summary files to.')
parser.add_argument('-v', '--verbose', action='store_true')
args = parser.parse_args()
if args.verbose:
logging.basicConfig(level=logging.INFO)
- ndjson_path = args.ndjson_path
- # TODO(agrieve): Remove above args once recipe is updated.
- if args.staging_dir:
- ndjson_path = os.path.join(args.staging_dir, _NDJSON_FILENAME)
-
logging.info('Creating Supersize diff')
supersize_diff_lines, delta_size_info = _CreateAndWriteSupersizeDiff(
- args.apk_name, args.before_dir, args.after_dir, args.supersize_diff_path)
+ args.apk_name, args.before_dir, args.after_dir)
changed_symbols = delta_size_info.raw_symbols.WhereDiffStatusIs(
models.DIFF_STATUS_UNCHANGED).Inverted()
@@ -193,8 +164,7 @@
# Monitor dex method growth since this correlates closely with APK size and
# may affect our dex file structure.
logging.info('Checking dex symbols')
- dex_delta_lines, dex_delta = _CreateAndWriteMethodCountDelta(
- changed_symbols, args.dex_method_count_diff_path)
+ dex_delta_lines, dex_delta = _CreateAndWriteMethodCountDelta(changed_symbols)
size_deltas = {dex_delta}
# Check for uncompressed .pak file entries being added to avoid unnecessary
@@ -206,56 +176,43 @@
logging.info('Creating sizes diff')
resource_sizes_lines, resource_sizes_delta = (
_CreateAndWriteResourceSizesDelta(args.apk_name, args.before_dir,
- args.after_dir,
- args.resource_sizes_diff_path))
+ args.after_dir))
size_deltas.add(resource_sizes_delta)
# .ndjson can be consumed by the html viewer.
logging.info('Creating HTML Report')
- html_report.BuildReportFromSizeInfo(
- ndjson_path, delta_size_info, all_symbols=True)
+ ndjson_path = os.path.join(args.staging_dir, _NDJSON_FILENAME)
+ html_report.BuildReportFromSizeInfo(ndjson_path, delta_size_info)
passing_deltas = set(m for m in size_deltas if m.IsAllowable())
failing_deltas = size_deltas - passing_deltas
is_roller = '-autoroll' in args.author
- checks_text = """
-
-Binary size checks {}.
-
-*******************************************************************************
+ failing_checks_text = '\n'.join(d.explanation for d in sorted(failing_deltas))
+ passing_checks_text = '\n'.join(d.explanation for d in sorted(passing_deltas))
+ checks_text = """\
FAILING:
-
{}
-*******************************************************************************
-
PASSING:
-
{}
-
-*******************************************************************************
-
-""".format('failed' if failing_deltas else 'passed', '\n\n'.join(
- d.explanation for d in sorted(failing_deltas)), '\n\n'.join(
- d.explanation for d in sorted(passing_deltas)))
+""".format(failing_checks_text, passing_checks_text)
if failing_deltas:
checks_text += _FAILURE_GUIDANCE
status_code = 1 if failing_deltas and not is_roller else 0
- summary = '<br>'.join([
- '',
- 'Normalized apk size delta: {}'.format(resource_sizes_delta.actual),
- 'Dex method count delta: {}'.format(dex_delta.actual),
- ])
+ summary = '<br>' + '<br>'.join(resource_sizes_lines)
+ if 'Empty Resource Sizes Diff' in summary:
+ summary = '<br>No size metrics were affected.'
+ if failing_deltas:
+ summary += '<br><br>Failed Size Checks:<br>'
+ summary += failing_checks_text.replace('\n', '<br>')
+ summary += '<br>Look at "Size Assertion Results" for guidance.'
- # TODO(agrieve): Remove once recipe is updated: details, normalized_apk_size
results_json = {
'status_code': status_code,
'summary': summary,
- 'details': checks_text,
- 'normalized_apk_size': resource_sizes_delta.actual,
'archive_filenames': [_NDJSON_FILENAME],
'links': [
{
@@ -263,10 +220,6 @@
'lines': checks_text.splitlines(),
},
{
- 'name': '>>> Resource Sizes Diff (high-level metrics) <<<',
- 'lines': resource_sizes_lines,
- },
- {
'name': '>>> Dex Method Diff <<<',
'lines': dex_delta_lines,
},
