Add presubmit check to ensure newly submitted histogram suffixes have labels

Bug: 1009844
Change-Id: Ibf6b878061f30c93541f29dad41520cf399dc2c5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1830090Reviewed-by: Ilya Sherman <isherman@chromium.org>
Commit-Queue: Tina Lu <ltina@google.com>
Cr-Commit-Position: refs/heads/master@{#703092}

tools/metrics/histograms/extract_histograms.py

@@ -521,7 +521,12 @@ def _UpdateHistogramsWithSuffixes(tree, histograms):
     suffix_nodes = histogram_suffixes.getElementsByTagName(suffix_tag)
     suffix_labels = {}
     for suffix in suffix_nodes:
-      suffix_labels[suffix.getAttribute('name')] = suffix.getAttribute('label')
+      suffix_name = suffix.getAttribute('name')
+      if not suffix.hasAttribute('label'):
+        logging.error('suffix %s in histogram_suffixes %s should have a label',
+                      suffix_name, name)
+        have_errors = True
+      suffix_labels[suffix_name] = suffix.getAttribute('label')
     # Find owners list under current histogram_suffixes tag.
     owners, _ = _ExtractOwners(histogram_suffixes)
 

tools/metrics/histograms/extract_histograms_test.py

@@ -481,6 +481,34 @@ class ExtractHistogramsTest(unittest.TestCase):
     self.assertEquals('This is a summary with & and " and \'',
                       hists['Test.Histogram']['summary'])
 
+  def testNewSuffixWithoutLabel(self):
+    suffix_without_label = xml.dom.minidom.parseString("""
+<histogram-configuration>
+<histogram_suffixes_list>
+  <histogram_suffixes name="Suffixes" separator=".">
+    <suffix base="true" name="BaseSuffix"/>
+  </histogram_suffixes>
+</histogram_suffixes_list>
+</histogram-configuration>
+""")
+    _, have_errors = extract_histograms.ExtractHistogramsFromDom(
+        suffix_without_label)
+    self.assertTrue(have_errors)
+
+  def testNewSuffixWithLabel(self):
+    suffix_with_label = xml.dom.minidom.parseString("""
+<histogram-configuration>
+<histogram_suffixes_list>
+  <histogram_suffixes name="Suffixes" separator=".">
+    <suffix base="true" name="BaseSuffix" label="Base"/>
+  </histogram_suffixes>
+</histogram_suffixes_list>
+</histogram-configuration>
+""")
+    have_errors = extract_histograms. _UpdateHistogramsWithSuffixes(
+        suffix_with_label, {})
+    self.assertFalse(have_errors)
+
 if __name__ == "__main__":
   logging.basicConfig(level=logging.ERROR + 1)
   unittest.main()