[Telemetry] Introduce SummarizableValue. (issue 1313243003 by eakuefner@chromium.org)

5 views
Skip to first unread message

eakuefner@chromium.org via codereview.chromium.org

unread,
Aug 27, 2015, 2:33:47 PM8/27/15
to aio...@chromium.org, nedn...@google.com, sull...@chromium.org, chromium...@chromium.org, telemet...@chromium.org
Reviewers: aiolos, nednguyen, sullivan,

Message:
PTAL. This is a pretty direct refresh of Annie's CL, which at the time was
pretty much ready to go. Some notes for reviewers:

1. There's one TODO in SummarizableValue to enable the assert that checks
that
all SummarizableValues have an indicated improvement_direction. All
Telemetry
unittests pass with the assert enabled, but we need to update client code's
AddValue callsites, which I want to do in a forthcoming CL since this one is
already so large.
2. I've opened crbug.com/483646 about fixing serialization/deserialization
for
values, which links to a short design doc I wrote. You may notice that the
tests
having to do with AsDict/FromDict are a little messy, but I don't think
that the
tests Annie originally added are undertesting; they're just unavoidably in
the
wrong place (subclasses) because of the design issue that I'm going to fix.
3. I've opened crbug.com/525688 about rewriting Summary from scratch. You
would
expect that a value named Summarizable would receive special treatment in
Summary, which is currently hard or impossible based on the fact that
Summary is
spaghetti. This is what Annie's CL stalled on previously, but I don't think
keeping summarization as-is for now blocks this code.

Description:
[Telemetry] Introduce SummarizableValue.

This is a refresh of crrev.com/809393002, wherein
improvement_direction was added to Value.

This CL pulls out only the Telemetry-side changes
in an attempt to make this CL a bit smaller; the
next change will be to update Telemetry clients to
use this new field.

BUG=459450

Please review this at https://codereview.chromium.org/1313243003/

Base URL: https://chromium.googlesource.com/chromium/src.git@master

Affected files (+602, -247 lines):
M
tools/telemetry/telemetry/internal/results/chart_json_output_formatter_unittest.py
M
tools/telemetry/telemetry/internal/results/csv_pivot_table_output_formatter_unittest.py
M
tools/telemetry/telemetry/internal/results/html_output_formatter_unittest.py
M
tools/telemetry/telemetry/internal/results/json_output_formatter_unittest.py
M tools/telemetry/telemetry/internal/results/page_test_results_unittest.py
M tools/telemetry/telemetry/internal/results/story_run_unittest.py
M tools/telemetry/telemetry/internal/story_runner_unittest.py
M tools/telemetry/telemetry/value/histogram.py
M tools/telemetry/telemetry/value/histogram_unittest.py
A + tools/telemetry/telemetry/value/improvement_direction.py
M tools/telemetry/telemetry/value/list_of_scalar_values.py
M tools/telemetry/telemetry/value/list_of_scalar_values_unittest.py
M tools/telemetry/telemetry/value/merge_values_unittest.py
M tools/telemetry/telemetry/value/scalar.py
M tools/telemetry/telemetry/value/scalar_unittest.py
A tools/telemetry/telemetry/value/summarizable.py
M tools/telemetry/telemetry/value/summary_unittest.py
M tools/telemetry/telemetry/web_perf/metrics/blob_timeline.py
M tools/telemetry/telemetry/web_perf/metrics/gpu_timeline.py
M tools/telemetry/telemetry/web_perf/metrics/memory_timeline.py
M tools/telemetry/telemetry/web_perf/metrics/responsiveness_metric.py
M tools/telemetry/telemetry/web_perf/metrics/single_event.py
M tools/telemetry/telemetry/web_perf/metrics/smoothness.py
M
tools/telemetry/telemetry/web_perf/timeline_based_measurement_unittest.py


Reply all
Reply to author
Forward
0 new messages