Skip to content

Commit

Permalink
new TestingContext class makes code-tests simpler and less error-prone
Browse files Browse the repository at this point in the history
* we also have now a `fontbakery.codetesting` module
* ported many code-tests to the new style. More to be done in upcoming commits.

Overview of the benefits of this new approach:
* Automation: The TestingContext class automatically computes check conditions for us so that we can focus on the actual task of performing the code-tests
* Reliability: Code-test is much cleaner which makes it less likely to contain bugs.
* Clarity: One can now read the test-cases as if they were English sentences. (sort of "Literate programming" paradigm)
* Code-tests as "documentation": The purpose of the code-test is easier to grasp even for non-coders.
* Precision: Check being tested is specified with its explicit check-id, instead of its function-name (that hinted at the id, but imprecisely)

(PR #3005)
Co-authored by Felipe Corrêa da Silva Sanches <juca@members.fsf.org> and Lasse Fister <lasse@graphicore.de>
  • Loading branch information
graphicore authored Sep 3, 2020
1 parent 6ce05f0 commit 20923a5
Show file tree
Hide file tree
Showing 53 changed files with 1,078 additions and 759 deletions.
11 changes: 5 additions & 6 deletions Lib/fontbakery/callable.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,9 @@ def __init__(self, func):

def __repr__(self):
return'<{}:{}>'.format(type(self).__name__,
getattr(self, 'id',
getattr(self, 'name',
super(FontbakeryCallable, self).__repr__()
)))
getattr(self, 'id',
getattr(self, 'name',
super().__repr__() )))

@property
@cached_getter
Expand Down Expand Up @@ -133,7 +132,7 @@ def __init__(self,
description = None, # short text
documentation=None, # long text, markdown?
force=False):
super(FontBakeryCondition, self).__init__(func)
super().__init__(func)
# self.id = id
self.name = func.__name__ if name is None else name
self.description, self.documentation = get_doc_desc(func,
Expand Down Expand Up @@ -218,7 +217,7 @@ def __init__(self,
looking at an advancedMessage.
TODO: The naming is a bit odd.
"""
super(FontBakeryCheck, self).__init__(checkfunc)
super().__init__(checkfunc)
self.id = id
self.name = checkfunc.__name__ if name is None else name
self.conditions = conditions or []
Expand Down
23 changes: 14 additions & 9 deletions Lib/fontbakery/checkrunner.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,21 +160,21 @@ class APIViolationError(FontBakeryRunnerError):
def __init__(self, message, result, *args):
self.message = message
self.result = result
super(APIViolationError, self).__init__(message, *args)
super().__init__(message, *args)


class ProtocolViolationError(FontBakeryRunnerError):
def __init__(self, message, *args):
self.message = message
super(ProtocolViolationError, self).__init__(message, *args)
super().__init__(message, *args)


class FailedCheckError(FontBakeryRunnerError):
def __init__(self, error, *args):
message = f'Failed with {type(error).__name__}: {error}'
self.error = error
self.traceback = "".join(traceback.format_tb(error.__traceback__))
super(FailedCheckError, self).__init__(message, *args)
super().__init__(message, *args)


class FailedConditionError(FontBakeryRunnerError):
Expand All @@ -187,7 +187,7 @@ def __init__(self, condition, error, *args):
self.condition = condition
self.error = error
self.traceback = "".join(traceback.format_tb(error.__traceback__))
super(FailedConditionError, self).__init__(message, *args)
super().__init__(message, *args)


class MissingConditionError(FontBakeryRunnerError):
Expand All @@ -199,7 +199,7 @@ def __init__(self, condition_name, error, *args):
f' {type(error).__name__}: {error}')
self.error = error
self.traceback = "".join(traceback.format_tb(error.__traceback__))
super(MissingConditionError, self).__init__(message, *args)
super().__init__(message, *args)


class FailedDependenciesError(FontBakeryRunnerError):
Expand All @@ -209,7 +209,7 @@ def __init__(self, check, error, *args):
self.check = check
self.error = error
self.traceback = "".join(traceback.format_tb(error.__traceback__))
super(FailedDependenciesError, self).__init__(message, *args)
super().__init__(message, *args)


class SetupError(FontBakeryRunnerError):
Expand Down Expand Up @@ -249,6 +249,7 @@ def __init__(self, profile, values
, custom_order=None
, explicit_checks=None
, exclude_checks=None
, use_cache=True
):
# TODO: transform all iterables that are list like to tuples
# to make sure that they won't change anymore.
Expand Down Expand Up @@ -278,11 +279,16 @@ def __init__(self, profile, values
f'{message}')
self._values = values

self.use_cache = use_cache
self._cache = {
'conditions': {}
, 'order': None
}

def clearCache(self):
# no need to clear 'order' cache IMO
self._cache['conditions'] = {}

@property
def iterargs(self):
""" uses the singular name as key """
Expand Down Expand Up @@ -413,12 +419,11 @@ def _filter_condition_used_iterargs(self, name, iterargs):

def _get_condition(self, name, iterargs, path=None):
# conditions are evaluated lazily
usecache = True #False
used_iterargs = self._filter_condition_used_iterargs(name, iterargs)
key = (name, used_iterargs)
if not usecache or key not in self._cache['conditions']:
if not self.use_cache or key not in self._cache['conditions']:
err, val = self._evaluate_condition(name, used_iterargs, path)
if usecache:
if self.use_cache:
self._cache['conditions'][key] = err, val
else:
err, val = self._cache['conditions'][key]
Expand Down
174 changes: 174 additions & 0 deletions Lib/fontbakery/codetesting.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,174 @@
#!/usr/bin/env python3
# Copyright 2020 The Fontbakery Authors
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#

from fontbakery.checkrunner import CheckRunner, Profile, get_module_profile


class TestingContext:
""" CAUTION: this uses a lot of "private" methods and properties
of CheckRunner, in order to make unit testing different cases simpler.
This is not intended to run in production. However, if that is desired
we may or may not find inspiration here on how to implement a proper
public CheckRunner API.
Not built for performance either!
The idea is that we can let this class take care of computing
the dependencies of a check for us. And we can also optionaly "fake"
some of them in order to create useful testing scenarios for the checks.
An initial run can be with unaltered arguments, as CheckRunner would
produce them by itself. And subsequent calls can reuse some of them.
Example:
> import fontbakery.profiles.googlefonts as googlefonts_profile
> check = TestingContext(googlefonts_profile,
'com.google.fonts/check/some_id')
> first_result = check(initialTTFont) # can be called with overrides
> modifiedTTFont = check['ttFont']
> mofifiedTTFont[opentype_table].some_field = some_value
> second_result = check(modifiedTTFont)
> overriden_value = check['some_dependency']
> overriden_value.change_something()
> another_result = check(modifiedTTFont, {'some_dependency': overriden_value})
"""
def __init__(self, module_or_profile, check_id):
self.profile = module_or_profile \
if isinstance(module_or_profile, Profile) \
else get_module_profile(module_or_profile)
self.check_id = check_id
self.check_identity = None
self.check_section = None
self.check = None
self.check_iterargs = None
self._args = None

def _get_args(self, condition_overrides=None):
if condition_overrides is not None:
for name_key, value in condition_overrides.items():
if isinstance(name_key, str):
# this is a simplified form of a cache key:
# write the conditions directly to the iterargs of the check identity
used_iterargs = self.runner._filter_condition_used_iterargs(name_key, self.check_iterargs)
key = (name_key, used_iterargs)
else:
# Full control for the caller, who has to inspect how
# the desired key needs to be set up.
key = name_key
# error, value
self.runner._cache['conditions'][key] = None, value
args = self.runner._get_args(self.check, self.check_iterargs)
# args that are derived iterables are generators that must be
# converted to lists, otherwise we end up with exhausted
# generators after their first consumption.
for k in args:
if self.profile.get_type(k, None) == 'derived_iterables':
args[k] = list(args[k])
return args

def __getitem__(self, key):
if key in self._args:
return self._args[key]

used_iterargs = self.runner._filter_condition_used_iterargs(key, self.check_iterargs)
key = (key, used_iterargs)
if key in self.runner._cache['conditions']:
return self.runner._cache['conditions'][key][1]

def __call__(self, values, condition_overrides={}):
from fontTools.ttLib import TTFont
if isinstance(values, str):
values = {'font': values,
'fonts': [values]}
elif isinstance(values, TTFont):
values = {'font': values.reader.file.name,
'fonts': [values.reader.file.name],
'ttFont': values,
'ttFonts': [values]}
elif isinstance(values, list):
if isinstance(values[0], str):
values = {'fonts': values}
elif isinstance(values[0], TTFont):
values = {'fonts': [v.reader.file.name for v in values],
'ttFonts': values}

self.runner = CheckRunner(self.profile, values, explicit_checks=[self.check_id])
for check_identity in self.runner.order:
_, check, _ = check_identity
if check.id != self.check_id:
continue
self.check_identity = check_identity
self.check_section, self.check, self.check_iterargs = check_identity
break
if self.check_identity is None:
raise KeyError(f'Check with id "{self.check_id}" not found.')

self._args = self._get_args(condition_overrides)
return list(self.runner._exec_check(self.check, self._args))


def portable_path(p):
import os
return os.path.join(*p.split('/'))


def TEST_FILE(f):
return portable_path("data/test/" + f)


def assert_PASS(check_results, reason="with a good font..."):
from fontbakery.checkrunner import PASS
print(f"Test PASS {reason}")
status, message = list(check_results)[-1]
assert status == PASS
return str(message)


def assert_SKIP(check_results, reason=""):
from fontbakery.checkrunner import SKIP
print(f"Test SKIP {reason}")
status, message = list(check_results)[-1]
assert status == SKIP
return str(message)


def assert_results_contain(check_results, expected_status, expected_msgcode=None, reason=None):
"""
This helper function is useful when we want to make sure that
a certain log message is emited by a check but it can be in any
order among other log messages.
"""
from fontbakery.message import Message
if not reason:
reason = f"[{expected_msgcode}]"

print(f"Test {expected_status} {reason}")
check_results = list(check_results)
for status, msg in check_results:
if (status == expected_status and
expected_msgcode == None or
(isinstance(msg, Message) and msg.code == expected_msgcode)): # At some point we will make
# message keywords mandatory!
if isinstance(msg, Message):
return msg.message
else:
return msg # It is probably a plain python string

#if not found:
raise Exception(f"Expected to find {expected_status}, [code: {expected_msgcode}]\n"
f"But did not find it in:\n"
f"{check_results}")
1 change: 1 addition & 0 deletions Lib/fontbakery/fonts_profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"""
from fontbakery.checkrunner import Profile

from fontbakery.callable import FontBakeryExpectedValue as ExpectedValue

class FontsProfile(Profile):
Expand Down
3 changes: 3 additions & 0 deletions Lib/fontbakery/profiles/cff.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
# used to inform get_module_profile whether and how to create a profile
from fontbakery.fonts_profile import profile_factory # NOQA pylint: disable=unused-import

profile_imports = [
('.shared_conditions', ('is_cff', 'is_cff2'))
]

def _get_subr_bias(count):
if count < 1240:
Expand Down
2 changes: 1 addition & 1 deletion Lib/fontbakery/profiles/name.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from fontbakery.fonts_profile import profile_factory # NOQA pylint: disable=unused-import

profile_imports = [
('.shared_conditions', ('glyph_metrics_stats', ))
('.shared_conditions', ('glyph_metrics_stats', 'is_ttf', 'is_cff'))
]


Expand Down
4 changes: 4 additions & 0 deletions Lib/fontbakery/profiles/post.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
# used to inform get_module_profile whether and how to create a profile
from fontbakery.fonts_profile import profile_factory # NOQA pylint: disable=unused-import

profile_imports = [
('.shared_conditions', ('is_ttf', ))
]

@check(
id = 'com.google.fonts/check/family/underline_thickness',
rationale = """
Expand Down
2 changes: 1 addition & 1 deletion Lib/fontbakery/reporters/ghmarkdown.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
class GHMarkdownReporter(SerializeReporter):

def __init__(self, loglevels, **kwd):
super(GHMarkdownReporter, self).__init__(**kwd)
super().__init__(**kwd)
self.loglevels = loglevels


Expand Down
2 changes: 1 addition & 1 deletion Lib/fontbakery/reporters/html.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class HTMLReporter(fontbakery.reporters.serialize.SerializeReporter):
"""Renders a report as a HTML document."""

def __init__(self, loglevels, **kwd):
super(HTMLReporter, self).__init__(**kwd)
super().__init__(**kwd)
self.loglevels = loglevels

def get_html(self) -> str:
Expand Down
4 changes: 2 additions & 2 deletions Lib/fontbakery/reporters/serialize.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class SerializeReporter(FontbakeryReporter):

def __init__(self, collect_results_by=None
, **kwd):
super(SerializeReporter, self).__init__(**kwd)
super().__init__(**kwd)
self._results_by = collect_results_by
self._items = {}
self._doc = None
Expand All @@ -49,7 +49,7 @@ def _set_metadata(self, identity, item):
pass

def _register(self, event):
super(SerializeReporter, self)._register(event)
super()._register(event)
status, message, identity = event
section, check, iterargs = identity
key = self._get_key(identity)
Expand Down
Loading

0 comments on commit 20923a5

Please sign in to comment.