Add histograms presubmit check for UseCounter.h

Add histograms presubmit check for UseCounter.h

The UMA mapping in tools/metrics/histograms/histograms.xml should be updated
whenever a new entry is added to the UseCounter::Feature enum. A script already
exists to automatically update this mapping
(tools/metrics/histograms/update_use_counter_feature_enum.py).

This patch extends the script's library to include a function that checks whether
running the script would produce a change. The patch also implements a presubmit
check that runs this function when UseCounter.h has been modified, and outputs a
warning if a change is detected.

BUG=

Committed: https://crrev.com/3240362085e84fcb32d09866c965b914d7fe0528
Cr-Commit-Position: refs/heads/master@{#378902}

[suzyh_UTC10 (ex-contributor), 2016-02-29 05:26:48]

Description was changed from

==========
WIP: UseCounter/histogram presubmit check

BUG=
==========

to

==========
Add histograms presubmit check for UseCounter.h

The UMA mapping in tools/metrics/histograms/histograms.xml should be updated
whenever a new entry is added to the UseCounter::Feature enum. A script already
exists to automatically update this mapping
(tools/metrics/histograms/update_use_counter_feature_enum.py).

This patch extends the script's library to include a function that checks
whether
running the script would produce a change. The patch also implements a presubmit
check that runs this function when UseCounter.h has been modified, and outputs a
warning if a change is detected.

BUG=
==========

[suzyh_UTC10 (ex-contributor), 2016-02-29 05:31:18]

suzyh@chromium.org changed reviewers:
+ jochen@chromium.org

[jochen (gone - plz use gerrit), 2016-03-01 14:28:23]

lgtm, thank you!

[suzyh_UTC10 (ex-contributor), 2016-03-01 23:32:46]

suzyh@chromium.org changed reviewers:
+ isherman@chromium.org

[suzyh_UTC10 (ex-contributor), 2016-03-01 23:33:11]

+isherman for tools/metrics OWNERS

[Ilya Sherman, 2016-03-02 00:59:29]

Thanks!

https://codereview.chromium.org/1745913002/diff/40001/tools/metrics/histogram...
File tools/metrics/histograms/update_histogram_enum.py (right):

https://codereview.chromium.org/1745913002/diff/40001/tools/metrics/histogram...
tools/metrics/histograms/update_histogram_enum.py:163: new_xml =
print_style.GetPrintStyle().PrettyPrintNode(histograms_doc)
It looks like you copy/pasted lines 151-163 here.  Please factor out this shared
code into a helper function.

[suzyh_UTC10 (ex-contributor), 2016-03-02 03:23:49]

https://codereview.chromium.org/1745913002/diff/40001/tools/metrics/histogram...
File tools/metrics/histograms/update_histogram_enum.py (right):

https://codereview.chromium.org/1745913002/diff/40001/tools/metrics/histogram...
tools/metrics/histograms/update_histogram_enum.py:163: new_xml =
print_style.GetPrintStyle().PrettyPrintNode(histograms_doc)
On 2016/03/02 at 00:59:29, Ilya Sherman wrote:
> It looks like you copy/pasted lines 151-163 here.  Please factor out this
shared code into a helper function.

Done. It's a bit messy, but I haven't been able to think of a better way of
organizing it, given that UpdateHistogramEnum, ReadHistogramValues and
UpdateHistogramFromDict are all used by other code.

[Ilya Sherman, 2016-03-02 23:18:46]

https://codereview.chromium.org/1745913002/diff/40001/tools/metrics/histogram...
File tools/metrics/histograms/update_histogram_enum.py (right):

https://codereview.chromium.org/1745913002/diff/40001/tools/metrics/histogram...
tools/metrics/histograms/update_histogram_enum.py:163: new_xml =
print_style.GetPrintStyle().PrettyPrintNode(histograms_doc)
On 2016/03/02 03:23:49, suzyh wrote:
> On 2016/03/02 at 00:59:29, Ilya Sherman wrote:
> > It looks like you copy/pasted lines 151-163 here.  Please factor out this
> shared code into a helper function.
> 
> Done. It's a bit messy, but I haven't been able to think of a better way of
> organizing it, given that UpdateHistogramEnum, ReadHistogramValues and
> UpdateHistogramFromDict are all used by other code.

Thanks!  This is the structure that I was thinking of as well -- I agree that
it's a bit awkward, but IMO it's still nicer than duplicating the code =)

https://codereview.chromium.org/1745913002/diff/80001/tools/metrics/histogram...
File tools/metrics/histograms/update_histogram_enum.py (right):

https://codereview.chromium.org/1745913002/diff/80001/tools/metrics/histogram...
tools/metrics/histograms/update_histogram_enum.py:146: source_enum_values,
source_enum_path):
Please add documentation for this function.

https://codereview.chromium.org/1745913002/diff/80001/tools/metrics/histogram...
tools/metrics/histograms/update_histogram_enum.py:158: 
Optional nit: I'd omit this newline.

https://codereview.chromium.org/1745913002/diff/80001/tools/metrics/histogram...
tools/metrics/histograms/update_histogram_enum.py:192: HISTOGRAMS_PATH =
path_util.GetHistogramsFile()
nit: I think this can be moved into _GetOldAndUpdatedXml

[suzyh_UTC10 (ex-contributor), 2016-03-02 23:27:10]

https://codereview.chromium.org/1745913002/diff/80001/tools/metrics/histogram...
File tools/metrics/histograms/update_histogram_enum.py (right):

https://codereview.chromium.org/1745913002/diff/80001/tools/metrics/histogram...
tools/metrics/histograms/update_histogram_enum.py:192: HISTOGRAMS_PATH =
path_util.GetHistogramsFile()
On 2016/03/02 at 23:18:46, Ilya Sherman wrote:
> nit: I think this can be moved into _GetOldAndUpdatedXml

I contemplated this, but then _GetOldAndUpdatedXml would also have to return
HISTOGRAMS_PATH, because it is needed below. Which would you prefer?

[Ilya Sherman, 2016-03-02 23:35:31]

https://codereview.chromium.org/1745913002/diff/80001/tools/metrics/histogram...
File tools/metrics/histograms/update_histogram_enum.py (right):

https://codereview.chromium.org/1745913002/diff/80001/tools/metrics/histogram...
tools/metrics/histograms/update_histogram_enum.py:192: HISTOGRAMS_PATH =
path_util.GetHistogramsFile()
On 2016/03/02 23:27:10, suzyh wrote:
> On 2016/03/02 at 23:18:46, Ilya Sherman wrote:
> > nit: I think this can be moved into _GetOldAndUpdatedXml
> 
> I contemplated this, but then _GetOldAndUpdatedXml would also have to return
> HISTOGRAMS_PATH, because it is needed below. Which would you prefer?

Ah, I missed that.  I think it would be fine to promote HISTOGRAMS_PATH to be a
file-level constant, so that it'd be accessible from any method.

[suzyh_UTC10 (ex-contributor), 2016-03-02 23:49:17]

https://codereview.chromium.org/1745913002/diff/80001/tools/metrics/histogram...
File tools/metrics/histograms/update_histogram_enum.py (right):

https://codereview.chromium.org/1745913002/diff/80001/tools/metrics/histogram...
tools/metrics/histograms/update_histogram_enum.py:146: source_enum_values,
source_enum_path):
On 2016/03/02 at 23:18:46, Ilya Sherman wrote:
> Please add documentation for this function.

Done.

https://codereview.chromium.org/1745913002/diff/80001/tools/metrics/histogram...
tools/metrics/histograms/update_histogram_enum.py:158: 
On 2016/03/02 at 23:18:46, Ilya Sherman wrote:
> Optional nit: I'd omit this newline.

Done.

https://codereview.chromium.org/1745913002/diff/80001/tools/metrics/histogram...
tools/metrics/histograms/update_histogram_enum.py:192: HISTOGRAMS_PATH =
path_util.GetHistogramsFile()
On 2016/03/02 at 23:35:31, Ilya Sherman wrote:
> On 2016/03/02 23:27:10, suzyh wrote:
> > On 2016/03/02 at 23:18:46, Ilya Sherman wrote:
> > > nit: I think this can be moved into _GetOldAndUpdatedXml
> > 
> > I contemplated this, but then _GetOldAndUpdatedXml would also have to return
> > HISTOGRAMS_PATH, because it is needed below. Which would you prefer?
> 
> Ah, I missed that.  I think it would be fine to promote HISTOGRAMS_PATH to be
a file-level constant, so that it'd be accessible from any method.

Done.

[Ilya Sherman, 2016-03-02 23:52:05]

LGTM -- thanks!

https://codereview.chromium.org/1745913002/diff/130001/tools/metrics/histogra...
File tools/metrics/histograms/update_histogram_enum.py (right):

https://codereview.chromium.org/1745913002/diff/130001/tools/metrics/histogra...
tools/metrics/histograms/update_histogram_enum.py:23: HISTOGRAMS_PATH =
path_util.GetHistogramsFile()
nit: Please leave two blank lines above and below this line (unless some pretty
printer disagrees with me, in which case go with what the tool says :-})

[suzyh_UTC10 (ex-contributor), 2016-03-03 00:04:58]

The CQ bit was checked by suzyh@chromium.org

[suzyh_UTC10 (ex-contributor), 2016-03-03 00:04:59]

Thanks for the review!

https://codereview.chromium.org/1745913002/diff/130001/tools/metrics/histogra...
File tools/metrics/histograms/update_histogram_enum.py (right):

https://codereview.chromium.org/1745913002/diff/130001/tools/metrics/histogra...
tools/metrics/histograms/update_histogram_enum.py:23: HISTOGRAMS_PATH =
path_util.GetHistogramsFile()
On 2016/03/02 at 23:52:05, Ilya Sherman wrote:
> nit: Please leave two blank lines above and below this line (unless some
pretty printer disagrees with me, in which case go with what the tool says :-})

Done.

[suzyh_UTC10 (ex-contributor), 2016-03-03 00:04:59]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org,
isherman@chromium.org
Link to the patchset: https://codereview.chromium.org/1745913002/#ps150001
(title: "Style fix")

[commit-bot: I haz the power, 2016-03-03 00:05:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1745913002/150001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1745913002/150001

[commit-bot: I haz the power, 2016-03-03 01:11:43]

Committed patchset #9 (id:150001)

[commit-bot: I haz the power, 2016-03-03 01:13:22]

Description was changed from

==========
Add histograms presubmit check for UseCounter.h

The UMA mapping in tools/metrics/histograms/histograms.xml should be updated
whenever a new entry is added to the UseCounter::Feature enum. A script already
exists to automatically update this mapping
(tools/metrics/histograms/update_use_counter_feature_enum.py).

This patch extends the script's library to include a function that checks
whether
running the script would produce a change. The patch also implements a presubmit
check that runs this function when UseCounter.h has been modified, and outputs a
warning if a change is detected.

BUG=
==========

to

==========
Add histograms presubmit check for UseCounter.h

The UMA mapping in tools/metrics/histograms/histograms.xml should be updated
whenever a new entry is added to the UseCounter::Feature enum. A script already
exists to automatically update this mapping
(tools/metrics/histograms/update_use_counter_feature_enum.py).

This patch extends the script's library to include a function that checks
whether
running the script would produce a change. The patch also implements a presubmit
check that runs this function when UseCounter.h has been modified, and outputs a
warning if a change is detected.

BUG=

Committed: https://crrev.com/3240362085e84fcb32d09866c965b914d7fe0528
Cr-Commit-Position: refs/heads/master@{#378902}
==========

[commit-bot: I haz the power, 2016-03-03 01:13:23]

Patchset 9 (id:??) landed as
https://crrev.com/3240362085e84fcb32d09866c965b914d7fe0528
Cr-Commit-Position: refs/heads/master@{#378902}