Bug 1482912 - Replace cpp_guard with operating_systems for Events and Scalars r=chutten
authorJan-Erik Rediger <jrediger@mozilla.com>
Tue, 22 Jan 2019 16:07:00 +0000
changeset 514837 9e9085aee8bcf0fd6fc9b71cb30bf02eab563478
parent 514836 d27e22f625f26ba6ae485d5da99f4b5ded52e065
child 514838 d762822c538bf374820e7c63c3b5658f297b1d78
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerschutten
bugs1482912
milestone66.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1482912 - Replace cpp_guard with operating_systems for Events and Scalars r=chutten Differential Revision: https://phabricator.services.mozilla.com/D17231
toolkit/components/telemetry/Scalars.yaml
toolkit/components/telemetry/build_scripts/gen_event_enum.py
toolkit/components/telemetry/build_scripts/gen_scalar_data.py
toolkit/components/telemetry/build_scripts/gen_scalar_enum.py
toolkit/components/telemetry/build_scripts/mozparsers/parse_events.py
toolkit/components/telemetry/build_scripts/mozparsers/parse_scalars.py
toolkit/components/telemetry/docs/collection/events.rst
toolkit/components/telemetry/docs/collection/scalars.rst
toolkit/components/telemetry/tests/python/python.ini
toolkit/components/telemetry/tests/python/test_parse_events.py
toolkit/components/telemetry/tests/python/test_parse_scalars.py
--- a/toolkit/components/telemetry/Scalars.yaml
+++ b/toolkit/components/telemetry/Scalars.yaml
@@ -14,17 +14,16 @@ a11y:
     kind: string
     notification_emails:
       - accessibility@mozilla.com
       - jteh@mozilla.com
     record_in_processes:
       - 'main'
     release_channel_collection: opt-out
     keyed: false
-    cpp_guard: 'ACCESSIBILITY'
 
   indicator_acted_on:
     bug_numbers:
       - 1412358
     description: >
       Recorded on click or SPACE/ENTER keypress event. Boolean stating if the
       accessibility indicactor button was acted on.
     expires: "62"
@@ -467,17 +466,18 @@ sandbox:
     expires: "62"
     kind: boolean
     keyed: true
     notification_emails:
       - bowen@mozilla.com
     release_channel_collection: opt-out
     record_in_processes:
       - main
-    cpp_guard: 'XP_WIN'
+    operating_systems:
+      - "windows"
 
 preferences:
   created_new_user_prefs_file:
     bug_numbers:
       - 1367813
     description: >-
       A boolean indicating that profile/prefs.js was not found and it is being
       created for the first time in this session.
--- a/toolkit/components/telemetry/build_scripts/gen_event_enum.py
+++ b/toolkit/components/telemetry/build_scripts/gen_event_enum.py
@@ -6,16 +6,17 @@
 #
 # The events are defined in files provided as command-line arguments.
 
 from __future__ import print_function
 from mozparsers.shared_telemetry_utils import ParserError
 from mozparsers import parse_events
 
 import sys
+import buildconfig
 
 banner = """/* This file is auto-generated, see gen_event_enum.py.  */
 """
 
 file_header = """\
 #ifndef mozilla_TelemetryEventEnums_h
 #define mozilla_TelemetryEventEnums_h
 namespace mozilla {
@@ -57,23 +58,19 @@ def main(output, *filenames):
 
     for category, indexed in grouped.iteritems():
         category_cpp = indexed[0][1].category_cpp
 
         print("// category: %s" % category, file=output)
         print("enum class %s : uint32_t {" % category_cpp, file=output)
 
         for event_index, e in indexed:
-            cpp_guard = e.cpp_guard
-            if cpp_guard:
-                print("#if defined(%s)" % cpp_guard, file=output)
-            for offset, label in enumerate(e.enum_labels):
-                print("  %s = %d," % (label, event_index + offset), file=output)
-            if cpp_guard:
-                print("#endif", file=output)
+            if e.record_on_os(buildconfig.substs["OS_TARGET"]):
+                for offset, label in enumerate(e.enum_labels):
+                    print("  %s = %d," % (label, event_index + offset), file=output)
 
         print("};\n", file=output)
 
     print("#if defined(_MSC_VER) && !defined(__clang__)", file=output)
     print("const uint32_t EventCount = %d;" % index, file=output)
     print("#else", file=output)
     print("constexpr uint32_t EventCount = %d;" % index, file=output)
     print("#endif\n", file=output)
--- a/toolkit/components/telemetry/build_scripts/gen_scalar_data.py
+++ b/toolkit/components/telemetry/build_scripts/gen_scalar_data.py
@@ -11,16 +11,17 @@ from mozparsers.shared_telemetry_utils i
     StringTable,
     static_assert,
     ParserError
 )
 from mozparsers import parse_scalars
 
 import json
 import sys
+import buildconfig
 
 # The banner/text at the top of the generated file.
 banner = """/* This file is auto-generated, only for internal use in TelemetryScalar.h,
    see gen_scalar_data.py. */
 """
 
 file_header = """\
 #ifndef mozilla_TelemetryScalarData_h
@@ -38,34 +39,28 @@ file_footer = """\
 def write_scalar_info(scalar, output, name_index, expiration_index, store_index, store_count):
     """Writes a scalar entry to the output file.
 
     :param scalar: a ScalarType instance describing the scalar.
     :param output: the output stream.
     :param name_index: the index of the scalar name in the strings table.
     :param expiration_index: the index of the expiration version in the strings table.
     """
-    cpp_guard = scalar.cpp_guard
-    if cpp_guard:
-        print("#if defined(%s)" % cpp_guard, file=output)
-
-    print("  {{ {}, {}, {}, {}, {}, {}, {}, {}, {} }},"
-          .format(scalar.nsITelemetry_kind,
-                  name_index,
-                  expiration_index,
-                  scalar.dataset,
-                  " | ".join(scalar.record_in_processes_enum),
-                  "true" if scalar.keyed else "false",
-                  " | ".join(scalar.products_enum),
-                  store_count,
-                  store_index),
-          file=output)
-
-    if cpp_guard:
-        print("#endif", file=output)
+    if scalar.record_on_os(buildconfig.substs["OS_TARGET"]):
+        print("  {{ {}, {}, {}, {}, {}, {}, {}, {}, {} }},"
+              .format(scalar.nsITelemetry_kind,
+                      name_index,
+                      expiration_index,
+                      scalar.dataset,
+                      " | ".join(scalar.record_in_processes_enum),
+                      "true" if scalar.keyed else "false",
+                      " | ".join(scalar.products_enum),
+                      store_count,
+                      store_index),
+              file=output)
 
 
 def write_scalar_tables(scalars, output):
     """Writes the scalar and strings tables to an header file.
 
     :param scalars: a list of ScalarType instances describing the scalars.
     :param output: the output stream.
     """
--- a/toolkit/components/telemetry/build_scripts/gen_scalar_enum.py
+++ b/toolkit/components/telemetry/build_scripts/gen_scalar_enum.py
@@ -7,16 +7,17 @@
 #
 # The scalars are defined in files provided as command-line arguments.
 
 from __future__ import print_function
 from mozparsers.shared_telemetry_utils import ParserError
 from mozparsers import parse_scalars
 
 import sys
+import buildconfig
 
 banner = """/* This file is auto-generated, see gen_scalar_enum.py.  */
 """
 
 file_header = """\
 #ifndef mozilla_TelemetryScalarEnums_h
 #define mozilla_TelemetryScalarEnums_h
 namespace mozilla {
@@ -43,22 +44,18 @@ def main(output, *filenames):
         print("\nError processing scalars:\n" + str(ex) + "\n")
         sys.exit(1)
 
     # Write the enum file.
     print(banner, file=output)
     print(file_header, file=output)
 
     for s in scalars:
-        cpp_guard = s.cpp_guard
-        if cpp_guard:
-            print("#if defined(%s)" % cpp_guard, file=output)
-        print("  %s," % s.enum_label, file=output)
-        if cpp_guard:
-            print("#endif", file=output)
+        if s.record_on_os(buildconfig.substs["OS_TARGET"]):
+            print("  %s," % s.enum_label, file=output)
 
     print("  ScalarCount,", file=output)
 
     print(file_footer, file=output)
 
 
 if __name__ == '__main__':
     main(sys.stdout, *sys.argv[1:])
--- a/toolkit/components/telemetry/build_scripts/mozparsers/parse_events.py
+++ b/toolkit/components/telemetry/build_scripts/mozparsers/parse_events.py
@@ -119,16 +119,17 @@ def type_check_event_fields(identifier, 
         'description': AtomicTypeChecker(basestring),
     }
     OPTIONAL_FIELDS = {
         'methods': ListTypeChecker(basestring),
         'release_channel_collection': AtomicTypeChecker(basestring),
         'expiry_version': AtomicTypeChecker(basestring),
         'extra_keys': DictTypeChecker(basestring, basestring),
         'products': ListTypeChecker(basestring),
+        'operating_systems': ListTypeChecker(basestring),
     }
     ALL_FIELDS = REQUIRED_FIELDS.copy()
     ALL_FIELDS.update(OPTIONAL_FIELDS)
 
     # Check that all the required fields are available.
     missing_fields = [f for f in REQUIRED_FIELDS.keys() if f not in definition]
     if len(missing_fields) > 0:
         ParserError(identifier + ': Missing required fields: ' + ', '.join(missing_fields)
@@ -197,16 +198,23 @@ class EventData:
 
         # Check products.
         products = definition.get('products', [])
         for product in products:
             if not utils.is_valid_product(product):
                 ParserError(self.identifier + ': Unknown value in products: ' +
                             product).handle_later()
 
+        # Check operating_systems.
+        operating_systems = definition.get('operating_systems', [])
+        for operating_system in operating_systems:
+            if not utils.is_valid_os(operating_system):
+                ParserError(self.identifier + ': Unknown value in operating_systems: ' +
+                            operating_system).handle_later()
+
         # Check extra_keys.
         extra_keys = definition.get('extra_keys', {})
         if len(extra_keys.keys()) > MAX_EXTRA_KEYS_COUNT:
             ParserError("%s: Number of extra_keys exceeds limit %d." %
                         (self.identifier, MAX_EXTRA_KEYS_COUNT)).handle_later()
         for key in extra_keys.iterkeys():
             string_check(self.identifier, field='extra_keys', value=key,
                          min_length=1, max_length=MAX_EXTRA_KEY_NAME_LENGTH,
@@ -271,18 +279,32 @@ class EventData:
         """Get the non-empty list of flags representing products to record data on"""
         return [utils.product_name_to_enum(p) for p in self.products]
 
     @property
     def expiry_version(self):
         return self._definition.get('expiry_version')
 
     @property
-    def cpp_guard(self):
-        return self._definition.get('cpp_guard')
+    def operating_systems(self):
+        """Get the list of operating systems to record data on"""
+        return self._definition.get('operating_systems', ["all"])
+
+    def record_on_os(self, target_os):
+        """Check if this probe should be recorded on the passed os."""
+        os = self.operating_systems
+        if "all" in os:
+            return True
+
+        canonical_os = utils.canonical_os(target_os)
+
+        if "unix" in os and canonical_os in utils.UNIX_LIKE_OS:
+            return True
+
+        return canonical_os in os
 
     @property
     def enum_labels(self):
         def enum(method_name, object_name):
             m = convert_to_cpp_identifier(method_name, "_")
             o = convert_to_cpp_identifier(object_name, "_")
             return m + '_' + o
         combinations = itertools.product(self.methods, self.objects)
--- a/toolkit/components/telemetry/build_scripts/mozparsers/parse_scalars.py
+++ b/toolkit/components/telemetry/build_scripts/mozparsers/parse_scalars.py
@@ -102,29 +102,30 @@ class ScalarType:
             'description': basestring,
             'expires': basestring,
             'kind': basestring,
             'notification_emails': list,  # This contains strings. See LIST_FIELDS_CONTENT.
             'record_in_processes': list,
         }
 
         OPTIONAL_FIELDS = {
-            'cpp_guard': basestring,
             'release_channel_collection': basestring,
             'keyed': bool,
+            'operating_systems': list,
             'products': list,
             'record_into_store': list,
         }
 
         # The types for the data within the fields that hold lists.
         LIST_FIELDS_CONTENT = {
             'bug_numbers': int,
             'notification_emails': basestring,
             'record_in_processes': basestring,
             'products': basestring,
+            'operating_systems': basestring,
             'record_into_store': basestring,
         }
 
         # Concatenate the required and optional field definitions.
         ALL_FIELDS = REQUIRED_FIELDS.copy()
         ALL_FIELDS.update(OPTIONAL_FIELDS)
 
         # Checks that all the required fields are available.
@@ -190,21 +191,23 @@ class ScalarType:
                         '.\nSee: {}'.format(BASE_DOC_URL)).handle_later()
 
         # Validate the collection policy.
         collection_policy = definition.get('release_channel_collection', None)
         if collection_policy and collection_policy not in ['opt-in', 'opt-out']:
             ParserError(self._name + ' - unknown collection policy: ' + collection_policy +
                         '.\nSee: {}#optional-fields'.format(BASE_DOC_URL)).handle_later()
 
-        # Validate the cpp_guard.
-        cpp_guard = definition.get('cpp_guard')
-        if cpp_guard and re.match(r'\W', cpp_guard):
-            ParserError(self._name + ' - invalid cpp_guard: ' + cpp_guard +
-                        '.\nSee: {}#optional-fields'.format(BASE_DOC_URL)).handle_later()
+        # Validate operating_systems.
+        operating_systems = definition.get('operating_systems', [])
+        for operating_system in operating_systems:
+            if not utils.is_valid_os(operating_system):
+                ParserError(self._name + ' - invalid entry in operating_systems: ' +
+                            operating_system +
+                            '.\nSee: {}#optional-fields'.format(BASE_DOC_URL)).handle_later()
 
         # Validate record_in_processes.
         record_in_processes = definition.get('record_in_processes', [])
         for proc in record_in_processes:
             if not utils.is_valid_process_name(proc):
                 ParserError(self._name + ' - unknown value in record_in_processes: ' + proc +
                             '.\nSee: {}'.format(BASE_DOC_URL)).handle_later()
 
@@ -322,19 +325,32 @@ class ScalarType:
     def dataset_short(self):
         """Get the short name of the chosen release channel collection policy for the scalar.
         """
         # The collection policy is optional, but we still define a default
         # behaviour for it.
         return self._definition.get('release_channel_collection', 'opt-in')
 
     @property
-    def cpp_guard(self):
-        """Get the cpp guard for this scalar"""
-        return self._definition.get('cpp_guard')
+    def operating_systems(self):
+        """Get the list of operating systems to record data on"""
+        return self._definition.get('operating_systems', ['all'])
+
+    def record_on_os(self, target_os):
+        """Check if this probe should be recorded on the passed os."""
+        os = self.operating_systems
+        if "all" in os:
+            return True
+
+        canonical_os = utils.canonical_os(target_os)
+
+        if "unix" in os and canonical_os in utils.UNIX_LIKE_OS:
+            return True
+
+        return canonical_os in os
 
     @property
     def record_into_store(self):
         """Get the list of stores this probe should be recorded into"""
         return self._definition.get('record_into_store', ['main'])
 
 
 def load_scalars(filename, strict_type_checks=True):
--- a/toolkit/components/telemetry/docs/collection/events.rst
+++ b/toolkit/components/telemetry/docs/collection/events.rst
@@ -128,16 +128,24 @@ The following event properties are valid
 
 - ``extra_keys`` *(optional, object)*: An object that specifies valid keys for the ``extra`` argument and a description - see the example above.
 - ``products`` *(optional, list of strings)*: A list of products the event can be recorded on. It defaults to ``all``. Currently supported values are:
 
   - ``firefox``
   - ``fennec``
   - ``geckoview``
   - ``all`` (record on all products)
+- ``operating_systems`` *(optional, list of strings)*: This field restricts recording to certain operating systems only. It defaults to ``all``. Currently supported values are:
+
+   - ``mac``
+   - ``linux``
+   - ``windows``
+   - ``android``
+   - ``unix``
+   - ``all`` (record on all operating systems)
 
 .. note::
 
   Combinations of ``category``, ``method``, and ``object`` defined in the file must be unique.
 
 The API
 =======
 
@@ -311,8 +319,9 @@ Version History
 - Firefox 58:
 
    - Ignore re-registering existing events for a category instead of failing (`bug 1408975 <https://bugzilla.mozilla.org/show_bug.cgi?id=1408975>`_).
    - Removed support for the ``expiry_date`` property, as it was unused (`bug 1414638 <https://bugzilla.mozilla.org/show_bug.cgi?id=1414638>`_).
 - Firefox 61:
 
    - Enabled support for adding events in artifact builds and build-faster workflows (`bug 1448945 <https://bugzilla.mozilla.org/show_bug.cgi?id=1448945>`_).
    - Added summarization of events (`bug 1440673 <https://bugzilla.mozilla.org/show_bug.cgi?id=1440673>`_).
+- Firefox 66: Replace ``cpp_guard`` with ``operating_systems`` (`bug 1482912 <https://bugzilla.mozilla.org/show_bug.cgi?id=1482912>`_)`
--- a/toolkit/components/telemetry/docs/collection/scalars.rst
+++ b/toolkit/components/telemetry/docs/collection/scalars.rst
@@ -158,27 +158,34 @@ Required Fields
   - ``content``;
   - ``gpu``;
   - ``all_children`` (record in all the child processes);
   - ``all`` (record in all the processes).
 
 Optional Fields
 ---------------
 
-- ``cpp_guard``: A string that gets inserted as an ``#ifdef`` directive around the automatically generated C++ declaration. This is typically used for platform-specific scalars, e.g. ``ANDROID``.
 - ``release_channel_collection``: This can be either ``opt-in`` (default) or ``opt-out``. With the former the scalar is submitted by default on pre-release channels, unless the user has opted out. With the latter the scalar is submitted by default on release and pre-release channels, unless the user has opted out.
 - ``keyed``: A boolean that determines whether this is a keyed scalar. It defaults to ``False``.
 - ``products``: A list of products the scalar can be recorded on. It defaults to ``all``. Currently supported values are:
 
   - ``firefox``
   - ``fennec``
   - ``geckoview``
   - ``all`` (record on all products)
 
 - ``record_into_store``: A list of stores this scalar should be recorded into. It defaults to ``[main]``.
+- ``operating_systems``: This field restricts recording to certain operating systems only. Use that in-place of previous ``cpp_guards`` to avoid inclusion on not-specified operating systems. It defaults to ``all``. Currently supported values are:
+
+   - ``mac``
+   - ``linux``
+   - ``windows``
+   - ``android``
+   - ``unix``
+   - ``all`` (record on all operating systems)
 
 String type restrictions
 ------------------------
 To prevent abuses, the content of a string scalar is limited to 50 characters in length. Trying
 to set a longer string will result in an error and no string being set.
 
 .. _scalars.keyed-scalars:
 
@@ -299,8 +306,9 @@ Version History
 - Firefox 51: Added keyed scalars (`bug 1277806 <https://bugzilla.mozilla.org/show_bug.cgi?id=1277806>`_).
 - Firefox 53: Added child process scalars (`bug 1278556 <https://bugzilla.mozilla.org/show_bug.cgi?id=1278556>`_).
 - Firefox 58
 
   - Added support for recording new scalars from add-ons (`bug 1393801 <bug https://bugzilla.mozilla.org/show_bug.cgi?id=1393801>`_).
   - Ignore re-registering existing scalars for a category instead of failing (`bug 1409323 <https://bugzilla.mozilla.org/show_bug.cgi?id=1409323>`_).
 
 - Firefox 60: Enabled support for adding scalars in artifact builds and build-faster workflows (`bug 1425909 <https://bugzilla.mozilla.org/show_bug.cgi?id=1425909>`_).
+- Firefox 66: Replace ``cpp_guard`` with ``operating_systems`` (`bug 1482912 <https://bugzilla.mozilla.org/show_bug.cgi?id=1482912>`_)`
--- a/toolkit/components/telemetry/tests/python/python.ini
+++ b/toolkit/components/telemetry/tests/python/python.ini
@@ -1,9 +1,10 @@
 [DEFAULT]
 skip-if = python == 3
 
 [test_gen_event_data_json.py]
 [test_gen_scalar_data_json.py]
 [test_histogramtools_non_strict.py]
 [test_histogramtools_strict.py]
+[test_parse_events.py]
 [test_parse_scalars.py]
 [test_usecounters.py]
new file mode 100644
--- /dev/null
+++ b/toolkit/components/telemetry/tests/python/test_parse_events.py
@@ -0,0 +1,106 @@
+# This Source Code Form is subject to the terms of Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+import yaml
+import mozunit
+import sys
+import unittest
+from os import path
+
+TELEMETRY_ROOT_PATH = path.abspath(path.join(path.dirname(__file__), path.pardir, path.pardir))
+sys.path.append(TELEMETRY_ROOT_PATH)
+# The parsers live in a subdirectory of "build_scripts", account for that.
+# NOTE: if the parsers are moved, this logic will need to be updated.
+sys.path.append(path.join(TELEMETRY_ROOT_PATH, "build_scripts"))
+from mozparsers.shared_telemetry_utils import ParserError
+from mozparsers import parse_events
+
+
+def load_event(event):
+    """Parse the passed event and return a dictionary
+
+    :param event: Event as YAML string
+    :returns: Parsed Event dictionary
+    """
+    return yaml.safe_load(event)
+
+
+class TestParser(unittest.TestCase):
+    def test_valid_event_defaults(self):
+        SAMPLE_EVENT = """
+objects: ["object1", "object2"]
+bug_numbers: [12345]
+notification_emails: ["test01@mozilla.com", "test02@mozilla.com"]
+record_in_processes: ["main"]
+description: This is a test entry for Telemetry.
+expiry_version: never
+"""
+        name = "test_event"
+        event = load_event(SAMPLE_EVENT)
+        evt = parse_events.EventData("CATEGORY",
+                                     name,
+                                     event,
+                                     strict_type_checks=True)
+        ParserError.exit_func()
+
+        self.assertEqual(evt.methods, [name])
+        self.assertEqual(evt.record_in_processes, ["main"])
+        self.assertEqual(evt.objects, ["object1", "object2"])
+        self.assertEqual(evt.products, ["all"])
+        self.assertEqual(evt.operating_systems, ["all"])
+        self.assertEqual(evt.extra_keys, [])
+
+    def test_wrong_collection(self):
+        SAMPLE_EVENT = """
+objects: ["object1", "object2"]
+bug_numbers: [12345]
+notification_emails: ["test01@mozilla.com", "test02@mozilla.com"]
+record_in_processes: ["main"]
+description: This is a test entry for Telemetry.
+expiry_version: never
+release_channel_collection: none
+"""
+        event = load_event(SAMPLE_EVENT)
+        parse_events.EventData("CATEGORY",
+                               "test_event",
+                               event,
+                               strict_type_checks=True)
+
+        self.assertRaises(SystemExit, ParserError.exit_func)
+
+    def test_valid_event_custom(self):
+        SAMPLE_EVENT = """
+methods: ["method1", "method2"]
+objects: ["object1", "object2"]
+bug_numbers: [12345]
+notification_emails: ["test01@mozilla.com", "test02@mozilla.com"]
+record_in_processes: ["content"]
+description: This is a test entry for Telemetry.
+expiry_version: never
+extra_keys:
+  key1: test1
+  key2: test2
+products:
+    - geckoview
+operating_systems:
+    - windows
+"""
+        name = "test_event"
+        event = load_event(SAMPLE_EVENT)
+        evt = parse_events.EventData("CATEGORY",
+                                     name,
+                                     event,
+                                     strict_type_checks=True)
+        ParserError.exit_func()
+
+        self.assertEqual(evt.methods, ["method1", "method2"])
+        self.assertEqual(evt.objects, ["object1", "object2"])
+        self.assertEqual(evt.record_in_processes, ["content"])
+        self.assertEqual(evt.products, ["geckoview"])
+        self.assertEqual(evt.operating_systems, ["windows"])
+        self.assertEqual(sorted(evt.extra_keys), ["key1", "key2"])
+
+
+if __name__ == '__main__':
+    mozunit.main()
--- a/toolkit/components/telemetry/tests/python/test_parse_scalars.py
+++ b/toolkit/components/telemetry/tests/python/test_parse_scalars.py
@@ -129,11 +129,75 @@ record_into_store: []
 """
         scalar = load_scalar(SAMPLE_SCALAR)
         parse_scalars.ScalarType("CATEGORY",
                                  "PROVE",
                                  scalar,
                                  strict_type_checks=True)
         self.assertRaises(SystemExit, ParserError.exit_func)
 
+    def test_operating_systems_default(self):
+        SAMPLE_SCALAR = """
+description: A nice one-line description.
+expires: never
+record_in_processes:
+  - 'main'
+kind: uint
+notification_emails:
+  - test01@mozilla.com
+bug_numbers:
+  - 12345
+"""
+        scalar = load_scalar(SAMPLE_SCALAR)
+        sclr = parse_scalars.ScalarType("CATEGORY",
+                                        "PROVE",
+                                        scalar,
+                                        strict_type_checks=True)
+        ParserError.exit_func()
+
+        self.assertEqual(sclr.operating_systems, ["all"])
+
+    def test_operating_systems_custom(self):
+        SAMPLE_SCALAR = """
+description: A nice one-line description.
+expires: never
+record_in_processes:
+  - 'main'
+kind: uint
+notification_emails:
+  - test01@mozilla.com
+bug_numbers:
+  - 12345
+operating_systems:
+    - windows
+"""
+        scalar = load_scalar(SAMPLE_SCALAR)
+        sclr = parse_scalars.ScalarType("CATEGORY",
+                                        "PROVE",
+                                        scalar,
+                                        strict_type_checks=True)
+        ParserError.exit_func()
+
+        self.assertEqual(sclr.operating_systems, ["windows"])
+
+    def test_operating_systems_empty(self):
+        SAMPLE_SCALAR = """
+description: A nice one-line description.
+expires: never
+record_in_processes:
+  - 'main'
+kind: uint
+notification_emails:
+  - test01@mozilla.com
+bug_numbers:
+  - 12345
+operating_systems: []
+"""
+        scalar = load_scalar(SAMPLE_SCALAR)
+        parse_scalars.ScalarType("CATEGORY",
+                                 "PROVE",
+                                 scalar,
+                                 strict_type_checks=True)
+        self.assertRaises(SystemExit, ParserError.exit_func)
+
 
 if __name__ == '__main__':
     mozunit.main()