Commit e47529a2 authored by Ken Rockot's avatar Ken Rockot Committed by Commit Bot

[mojom] Enum backward-compatibility checks

Implements a test for backward-compatibility between any two versions
of a mojom enum definition.

This will be used to enforce stability checks once a [Stable]
attribute is introduced.

Bug: 1070663
Change-Id: I84c3051337adeb10a8cef0eadd0708548215d0bf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2212890Reviewed-by: default avatarOksana Zhuravlova <oksamyt@chromium.org>
Commit-Queue: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#771985}
parent a8e68b9c
#!/usr/bin/env python
# Copyright 2020 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
......
#!/usr/bin/env python
# Copyright 2020 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
......
......@@ -964,6 +964,37 @@ class Enum(Kind):
return self.attributes.get(ATTRIBUTE_EXTENSIBLE, False) \
if self.attributes else False
def IsBackwardCompatible(self, older_enum):
"""This enum is backward-compatible with older_enum if and only if one of
the following conditions holds:
- Neither enum is [Extensible] and both have the exact same set of valid
numeric values. Field names and aliases for the same numeric value do
not affect compatibility.
- older_enum is [Extensible], and for every version defined by
older_enum, this enum has the exact same set of valid numeric values.
"""
def buildVersionFieldMap(enum):
fields_by_min_version = {}
for field in enum.fields:
if field.min_version not in fields_by_min_version:
fields_by_min_version[field.min_version] = set()
fields_by_min_version[field.min_version].add(field.numeric_value)
return fields_by_min_version
old_fields = buildVersionFieldMap(older_enum)
new_fields = buildVersionFieldMap(self)
if new_fields.keys() != old_fields.keys() and not older_enum.extensible:
return False
for min_version, valid_values in old_fields.items():
if (min_version not in new_fields
or new_fields[min_version] != valid_values):
return False
return True
def __eq__(self, rhs):
return (isinstance(rhs, Enum) and
(self.mojom_name, self.native_only, self.fields, self.attributes,
......
#!/usr/bin/env python
# Copyright 2020 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
......
# Copyright 2020 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
from mojom_parser_test_case import MojomParserTestCase
class VersionCompatibilityTest(MojomParserTestCase):
"""Tests covering compatibility between two versions of the same mojom type
definition. This coverage ensures that we can reliably detect unsafe changes
to definitions that are expected to tolerate version skew in production
environments."""
def _GetTypeCompatibilityMap(self, old_mojom, new_mojom):
"""Helper to support the implementation of assertBackwardCompatible and
assertNotBackwardCompatible."""
old = self.ExtractTypes(old_mojom)
new = self.ExtractTypes(new_mojom)
self.assertEqual(set(old.keys()), set(new.keys()),
'Old and new test mojoms should use the same type names.')
compatibility_map = {}
for name in old.keys():
compatibility_map[name] = new[name].IsBackwardCompatible(old[name])
return compatibility_map
def assertBackwardCompatible(self, old_mojom, new_mojom):
compatibility_map = self._GetTypeCompatibilityMap(old_mojom, new_mojom)
for name, compatible in compatibility_map.items():
if not compatible:
raise AssertionError(
'Given the old mojom:\n\n %s\n\nand the new mojom:\n\n %s\n\n'
'The new definition of %s should pass a backward-compatibiity '
'check, but it does not.' % (old_mojom, new_mojom, name))
def assertNotBackwardCompatible(self, old_mojom, new_mojom):
compatibility_map = self._GetTypeCompatibilityMap(old_mojom, new_mojom)
if all(compatibility_map.values()):
raise AssertionError(
'Given the old mojom:\n\n %s\n\nand the new mojom:\n\n %s\n\n'
'The new mojom should fail a backward-compatibility check, but it '
'does not.' % (old_mojom, new_mojom))
def testNewNonExtensibleEnumValue(self):
"""Adding a value to a non-extensible enum breaks backward-compatibility."""
self.assertNotBackwardCompatible('enum E { kFoo, kBar };',
'enum E { kFoo, kBar, kBaz };')
def testNewNonExtensibleEnumValueWithMinVersion(self):
"""Adding a value to a non-extensible enum breaks backward-compatibility,
even with a new [MinVersion] specified for the value."""
self.assertNotBackwardCompatible(
'enum E { kFoo, kBar };', 'enum E { kFoo, kBar, [MinVersion=1] kBaz };')
def testNewValueInExistingVersion(self):
"""Adding a value to an existing version is not allowed, even if the old
enum was marked [Extensible]. Note that it is irrelevant whether or not the
new enum is marked [Extensible]."""
self.assertNotBackwardCompatible('[Extensible] enum E { kFoo, kBar };',
'enum E { kFoo, kBar, kBaz };')
self.assertNotBackwardCompatible(
'[Extensible] enum E { kFoo, kBar };',
'[Extensible] enum E { kFoo, kBar, kBaz };')
self.assertNotBackwardCompatible(
'[Extensible] enum E { kFoo, [MinVersion=1] kBar };',
'enum E { kFoo, [MinVersion=1] kBar, [MinVersion=1] kBaz };')
def testEnumValueRemoval(self):
"""Removal of an enum value is never valid even for [Extensible] enums."""
self.assertNotBackwardCompatible('enum E { kFoo, kBar };',
'enum E { kFoo };')
self.assertNotBackwardCompatible('[Extensible] enum E { kFoo, kBar };',
'[Extensible] enum E { kFoo };')
self.assertNotBackwardCompatible(
'[Extensible] enum E { kA, [MinVersion=1] kB };',
'[Extensible] enum E { kA, };')
self.assertNotBackwardCompatible(
'[Extensible] enum E { kA, [MinVersion=1] kB, [MinVersion=1] kZ };',
'[Extensible] enum E { kA, [MinVersion=1] kB };')
def testNewExtensibleEnumValueWithMinVersion(self):
"""Adding a new and properly [MinVersion]'d value to an [Extensible] enum
is a backward-compatible change. Note that it is irrelevant whether or not
the new enum is marked [Extensible]."""
self.assertBackwardCompatible('[Extensible] enum E { kA, kB };',
'enum E { kA, kB, [MinVersion=1] kC };')
self.assertBackwardCompatible(
'[Extensible] enum E { kA, kB };',
'[Extensible] enum E { kA, kB, [MinVersion=1] kC };')
self.assertBackwardCompatible(
'[Extensible] enum E { kA, [MinVersion=1] kB };',
'[Extensible] enum E { kA, [MinVersion=1] kB, [MinVersion=2] kC };')
def testRenameEnumValue(self):
"""Renaming an enum value does not affect backward-compatibility. Only
numeric value is relevant."""
self.assertBackwardCompatible('enum E { kA, kB };', 'enum E { kX, kY };')
def testAddEnumValueAlias(self):
"""Adding new enum fields does not affect backward-compatibility if it does
not introduce any new numeric values."""
self.assertBackwardCompatible(
'enum E { kA, kB };', 'enum E { kA, kB, kC = kA, kD = 1, kE = kD };')
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment