Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add bignum test case generation script #6093

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
8b2df74
Add bignum test generation framework
wernerlewis Jul 8, 2022
69a92ce
Add test generation for bignum cmp variant
wernerlewis Jul 18, 2022
86caf85
Add test case generation for bignum add
wernerlewis Jul 18, 2022
a51fe2b
Sort tests when generating cases
wernerlewis Jul 20, 2022
b17ca8a
Remove set() to preserve test case order
wernerlewis Jul 20, 2022
c442f6a
Fix type issues
wernerlewis Jul 20, 2022
265e051
Remove is None from if statement
wernerlewis Jul 20, 2022
6a31396
Fix incorrect indentation
wernerlewis Jul 20, 2022
75ef944
Fix CMake change failures on Windows
wernerlewis Jul 21, 2022
383461c
Separate CMake targets for bignum and PSA
wernerlewis Aug 23, 2022
fbb75e3
Separate common test generation classes/functions
wernerlewis Aug 24, 2022
55e638c
Remove abbreviations and clarify attributes
wernerlewis Aug 23, 2022
92c876a
Remove unneeded list concatenation
wernerlewis Aug 23, 2022
6c70d74
Convert bools to int before arithmetic
wernerlewis Aug 24, 2022
169034a
Add details to docstrings
wernerlewis Aug 23, 2022
699e126
Use ABCMeta for abstract classes
wernerlewis Aug 24, 2022
2b527a3
Split generate_tests to reduce code complexity
wernerlewis Aug 24, 2022
cfd4768
Use __new__() for case counting
wernerlewis Aug 24, 2022
d03d2a3
Remove trailing whitespace in description
wernerlewis Aug 24, 2022
6300b4f
Add missing typing
wernerlewis Aug 24, 2022
9990b30
Use typing casts for fixed-width tuples
wernerlewis Aug 24, 2022
a195ce7
Disable pylint unused arg in __new__
wernerlewis Aug 24, 2022
6d654c6
Raise NotImplementedError in abstract methods
wernerlewis Aug 25, 2022
e3ad22e
Fix TARGET types and code style
wernerlewis Aug 25, 2022
a16b617
Disable abstract check in pylint
wernerlewis Aug 25, 2022
f156c43
Use argparser default for directory
wernerlewis Aug 25, 2022
6ef5436
Clarify documentation
wernerlewis Aug 25, 2022
9df9faa
Use argparser default for targets
wernerlewis Aug 25, 2022
76f4562
Fix trailing whitespace
wernerlewis Aug 25, 2022
3366ebc
Add test_generation.py dependency in builds
wernerlewis Aug 25, 2022
81f2444
Modify wording in docstrings
wernerlewis Aug 25, 2022
a4b7720
Use `combinations_with_replacement` for inputs
wernerlewis Aug 31, 2022
466f036
Add dependencies attribute to BaseTarget
wernerlewis Aug 31, 2022
aaf3b79
Use Python 3.5 style typing for dependencies
wernerlewis Aug 31, 2022
a4668a6
Rework TestGenerator to add file targets
wernerlewis Sep 2, 2022
5601308
Remove unused imports
wernerlewis Sep 2, 2022
855e45c
Use simpler int to hex string conversion
wernerlewis Sep 2, 2022
1fade8a
Move symbol definition out of __init__
wernerlewis Sep 12, 2022
3dc4519
Replace L/R inputs with A/B
wernerlewis Sep 12, 2022
34d6d3e
Update comments/docstrings in TestGenerator
wernerlewis Sep 14, 2022
858cffd
Add toggle for test case count in descriptions
wernerlewis Sep 14, 2022
00d0242
Remove argparser default for directory
wernerlewis Sep 14, 2022
b6e8091
Use typing.cast instead of unqualified cast
wernerlewis Sep 14, 2022
ac446c8
Add combination_pairs helper function
wernerlewis Sep 14, 2022
52ae326
Update references to file targets in docstrings
wernerlewis Sep 14, 2022
07c830c
Fix setting for default test suite directory
wernerlewis Sep 15, 2022
c2fb540
Use a script specific description in CLI help
wernerlewis Sep 16, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions scripts/make_generated_files.bat
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,5 @@ perl scripts\generate_features.pl || exit /b 1
python scripts\generate_ssl_debug_helpers.py || exit /b 1
perl scripts\generate_visualc_files.pl || exit /b 1
python scripts\generate_psa_constants.py || exit /b 1
python tests\scripts\generate_bignum_tests.py || exit /b 1
python tests\scripts\generate_psa_tests.py || exit /b 1
219 changes: 219 additions & 0 deletions scripts/mbedtls_dev/test_generation.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,219 @@
"""Common test generation classes and main function.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only just realized, but we already have test code generation (tests/scripts/generate_test_code.py, turns .function files into .c), so “test generation” is ambiguous: we should specify that this is test data generation. I propose (in a follow-up) to rename this module to test_data_generation.


These are used both by generate_psa_tests.py and generate_bignum_tests.py.
"""

# Copyright The Mbed TLS Contributors
# SPDX-License-Identifier: Apache-2.0
#
# 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.

import argparse
import os
import posixpath
import re

from abc import ABCMeta, abstractmethod
from typing import Callable, Dict, Iterable, Iterator, List, Type, TypeVar

from mbedtls_dev import build_tree
from mbedtls_dev import test_case
Comment on lines +29 to +30
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should be relative imports. No reason to assume that scripts is in the search path.

Again, this is a preexisting bug which doesn't matter in our normal usage, so not a blocker.


T = TypeVar('T') #pylint: disable=invalid-name


class BaseTarget(metaclass=ABCMeta):
"""Base target for test case generation.

Child classes of this class represent an output file, and can be referred
to as file targets. These indicate where test cases will be written to for
all subclasses of the file target, which is set by `target_basename`.

Attributes:
count: Counter for test cases from this class.
case_description: Short description of the test case. This may be
automatically generated using the class, or manually set.
dependencies: A list of dependencies required for the test case.
show_test_count: Toggle for inclusion of `count` in the test description.
target_basename: Basename of file to write generated tests to. This
should be specified in a child class of BaseTarget.
test_function: Test function which the class generates cases for.
test_name: A common name or description of the test function. This can
be `test_function`, a clearer equivalent, or a short summary of the
test function's purpose.
"""
count = 0
case_description = ""
dependencies = [] # type: List[str]
show_test_count = True
target_basename = ""
test_function = ""
test_name = ""

def __new__(cls, *args, **kwargs):
# pylint: disable=unused-argument
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are moving to metaclasses, we could consider doing something more fancy than incrementing a counter.

We have two classes BaseTarget and TestGenerator which are mostly consumed by different modules, but in effect are doing the same thing. The could be merged into one class that works for everything, and optimised in consecutive pr's without having to change the consuming interfaces.

Methods like:

  • TestGenerator
  • write_test_data_file

are generic IO methods which can be common for both and BaseTarget.generate_target() TestGenerator.generate_tests() can be unified, or simply renamed.

Or for a more lazy approach we could even have a self.subtype == "PSA_Test_Target/ BigNum_Test_Target"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why we'd want anything fancier here.

Also please keep in mind that the average maintainer of this file is primarily a C programmer, and not a Python expert.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was more of a proposal as a stepping stone if we are aiming to intergrate those two into the future. Not a strong ask by any means ;)

cls.count += 1
return super().__new__(cls)

@abstractmethod
def arguments(self) -> List[str]:
"""Get the list of arguments for the test case.

Override this method to provide the list of arguments required for
the `test_function`.

Returns:
List of arguments required for the test function.
"""
raise NotImplementedError

def description(self) -> str:
"""Create a test case description.

Creates a description of the test case, including a name for the test
function, an optional case count, and a description of the specific
test case. This should inform a reader what is being tested, and
provide context for the test case.

Returns:
Description for the test case.
"""
if self.show_test_count:
return "{} #{} {}".format(
self.test_name, self.count, self.case_description
).strip()
else:
return "{} {}".format(self.test_name, self.case_description).strip()


def create_test_case(self) -> test_case.TestCase:
"""Generate TestCase from the instance."""
tc = test_case.TestCase()
tc.set_description(self.description())
tc.set_function(self.test_function)
tc.set_arguments(self.arguments())
tc.set_dependencies(self.dependencies)

return tc

@classmethod
@abstractmethod
def generate_function_tests(cls) -> Iterator[test_case.TestCase]:
"""Generate test cases for the class test function.

This will be called in classes where `test_function` is set.
Implementations should yield TestCase objects, by creating instances
of the class with appropriate input data, and then calling
`create_test_case()` on each.
"""
raise NotImplementedError

@classmethod
def generate_tests(cls) -> Iterator[test_case.TestCase]:
"""Generate test cases for the class and its subclasses.

In classes with `test_function` set, `generate_function_tests()` is
called to generate test cases first.

In all classes, this method will iterate over its subclasses, and
yield from `generate_tests()` in each. Calling this method on a class X
will yield test cases from all classes derived from X.
"""
if cls.test_function:
yield from cls.generate_function_tests()
for subclass in sorted(cls.__subclasses__(), key=lambda c: c.__name__):
yield from subclass.generate_tests()


class TestGenerator:
"""Generate test cases and write to data files."""
def __init__(self, options) -> None:
self.test_suite_directory = self.get_option(options, 'directory',
'tests/suites')
# Update `targets` with an entry for each child class of BaseTarget.
# Each entry represents a file generated by the BaseTarget framework,
# and enables generating the .data files using the CLI.
self.targets.update({
subclass.target_basename: subclass.generate_tests
for subclass in BaseTarget.__subclasses__()
})

@staticmethod
def get_option(options, name: str, default: T) -> T:
value = getattr(options, name, None)
return default if value is None else value

def filename_for(self, basename: str) -> str:
"""The location of the data file with the specified base name."""
return posixpath.join(self.test_suite_directory, basename + '.data')

def write_test_data_file(self, basename: str,
test_cases: Iterable[test_case.TestCase]) -> None:
"""Write the test cases to a .data file.

The output file is ``basename + '.data'`` in the test suite directory.
"""
filename = self.filename_for(basename)
test_case.write_data_file(filename, test_cases)

# Note that targets whose names contain 'test_format' have their content
# validated by `abi_check.py`.
targets = {} # type: Dict[str, Callable[..., Iterable[test_case.TestCase]]]

def generate_target(self, name: str, *target_args) -> None:
"""Generate cases and write to data file for a target.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent whitepace after function describing docstring


For target callables which require arguments, override this function
and pass these arguments using super() (see PSATestGenerator).
"""
test_cases = self.targets[name](*target_args)
self.write_test_data_file(name, test_cases)

def main(args, description: str, generator_class: Type[TestGenerator] = TestGenerator):
"""Command line entry point."""
parser = argparse.ArgumentParser(description=description)
parser.add_argument('--list', action='store_true',
help='List available targets and exit')
parser.add_argument('--list-for-cmake', action='store_true',
help='Print \';\'-separated list of available targets and exit')
parser.add_argument('--directory', metavar='DIR',
help='Output directory (default: tests/suites)')
# The `--directory` option is interpreted relative to the directory from
# which the script is invoked, but the default is relative to the root of
# the mbedtls tree. The default should not be set above, but instead after
# `build_tree.chdir_to_root()` is called.
Comment on lines +191 to +194
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, not quite. --directory is still interpreted relative to the root of the mbedtls tree. An abspath call is missing somewhere (or alternatively we could stop using chdir, but that would be annoying).

But since this is preexisting (I know it worked when I originally wrote it, but I might have broken it before I even committed), this is a non-blocker.

parser.add_argument('targets', nargs='*', metavar='TARGET',
help='Target file to generate (default: all; "-": none)')
options = parser.parse_args(args)
build_tree.chdir_to_root()
generator = generator_class(options)
if options.list:
for name in sorted(generator.targets):
print(generator.filename_for(name))
return
# List in a cmake list format (i.e. ';'-separated)
if options.list_for_cmake:
print(';'.join(generator.filename_for(name)
for name in sorted(generator.targets)), end='')
return
if options.targets:
# Allow "-" as a special case so you can run
# ``generate_xxx_tests.py - $targets`` and it works uniformly whether
# ``$targets`` is empty or not.
options.targets = [os.path.basename(re.sub(r'\.data\Z', r'', target))
for target in options.targets
if target != '-']
else:
options.targets = sorted(generator.targets)
for target in options.targets:
generator.generate_target(target)
70 changes: 59 additions & 11 deletions tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,17 @@ endif()
file(MAKE_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/suites)

# Get base names for generated files (starting at "suites/")
execute_process(
COMMAND
${MBEDTLS_PYTHON_EXECUTABLE}
${CMAKE_CURRENT_SOURCE_DIR}/../tests/scripts/generate_bignum_tests.py
--list-for-cmake
--directory suites
WORKING_DIRECTORY
${CMAKE_CURRENT_SOURCE_DIR}/..
OUTPUT_VARIABLE
base_bignum_generated_data_files)

execute_process(
COMMAND
${MBEDTLS_PYTHON_EXECUTABLE}
Expand All @@ -26,18 +37,37 @@ execute_process(
WORKING_DIRECTORY
${CMAKE_CURRENT_SOURCE_DIR}/..
OUTPUT_VARIABLE
base_generated_data_files)
base_psa_generated_data_files)

# Derive generated file paths in the build directory
set(generated_data_files "")
foreach(file ${base_generated_data_files})
list(APPEND generated_data_files ${CMAKE_CURRENT_BINARY_DIR}/${file})
set(base_generated_data_files ${base_bignum_generated_data_files} ${base_psa_generated_data_files})
set(bignum_generated_data_files "")
set(psa_generated_data_files "")
foreach(file ${base_bignum_generated_data_files})
list(APPEND bignum_generated_data_files ${CMAKE_CURRENT_BINARY_DIR}/${file})
endforeach()
foreach(file ${base_psa_generated_data_files})
list(APPEND psa_generated_data_files ${CMAKE_CURRENT_BINARY_DIR}/${file})
endforeach()

if(GEN_FILES)
add_custom_command(
OUTPUT
${generated_data_files}
${bignum_generated_data_files}
WORKING_DIRECTORY
${CMAKE_CURRENT_SOURCE_DIR}/..
COMMAND
${MBEDTLS_PYTHON_EXECUTABLE}
${CMAKE_CURRENT_SOURCE_DIR}/../tests/scripts/generate_bignum_tests.py
--directory ${CMAKE_CURRENT_BINARY_DIR}/suites
DEPENDS
${CMAKE_CURRENT_SOURCE_DIR}/../tests/scripts/generate_bignum_tests.py
${CMAKE_CURRENT_SOURCE_DIR}/../scripts/mbedtls_dev/test_case.py
${CMAKE_CURRENT_SOURCE_DIR}/../scripts/mbedtls_dev/test_generation.py
)
add_custom_command(
OUTPUT
${psa_generated_data_files}
WORKING_DIRECTORY
${CMAKE_CURRENT_SOURCE_DIR}/..
COMMAND
gilles-peskine-arm marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -50,6 +80,7 @@ if(GEN_FILES)
${CMAKE_CURRENT_SOURCE_DIR}/../scripts/mbedtls_dev/macro_collector.py
${CMAKE_CURRENT_SOURCE_DIR}/../scripts/mbedtls_dev/psa_storage.py
${CMAKE_CURRENT_SOURCE_DIR}/../scripts/mbedtls_dev/test_case.py
${CMAKE_CURRENT_SOURCE_DIR}/../scripts/mbedtls_dev/test_generation.py
${CMAKE_CURRENT_SOURCE_DIR}/../include/psa/crypto_config.h
${CMAKE_CURRENT_SOURCE_DIR}/../include/psa/crypto_values.h
${CMAKE_CURRENT_SOURCE_DIR}/../include/psa/crypto_extra.h
Expand All @@ -65,7 +96,8 @@ endif()
# they can cause race conditions in parallel builds.
# With this line, only 4 sub-makefiles include the above command, that reduces
# the risk of a race.
add_custom_target(test_suite_generated_data DEPENDS ${generated_data_files})
add_custom_target(test_suite_bignum_generated_data DEPENDS ${bignum_generated_data_files})
add_custom_target(test_suite_psa_generated_data DEPENDS ${psa_generated_data_files})
# Test suites caught by SKIP_TEST_SUITES are built but not executed.
# "foo" as a skip pattern skips "test_suite_foo" and "test_suite_foo.bar"
# but not "test_suite_foobar".
Expand All @@ -82,23 +114,39 @@ function(add_test_suite suite_name)

# Get the test names of the tests with generated .data files
# from the generated_data_files list in parent scope.
set(generated_data_names "")
foreach(generated_data_file ${generated_data_files})
set(bignum_generated_data_names "")
set(psa_generated_data_names "")
foreach(generated_data_file ${bignum_generated_data_files})
# Get the plain filename
get_filename_component(generated_data_name ${generated_data_file} NAME)
# Remove the ".data" extension
get_name_without_last_ext(generated_data_name ${generated_data_name})
# Remove leading "test_suite_"
string(SUBSTRING ${generated_data_name} 11 -1 generated_data_name)
list(APPEND bignum_generated_data_names ${generated_data_name})
endforeach()
foreach(generated_data_file ${psa_generated_data_files})
# Get the plain filename
get_filename_component(generated_data_name ${generated_data_file} NAME)
# Remove the ".data" extension
get_name_without_last_ext(generated_data_name ${generated_data_name})
# Remove leading "test_suite_"
string(SUBSTRING ${generated_data_name} 11 -1 generated_data_name)
list(APPEND generated_data_names ${generated_data_name})
list(APPEND psa_generated_data_names ${generated_data_name})
endforeach()

if(";${generated_data_names};" MATCHES ";${data_name};")
if(";${bignum_generated_data_names};" MATCHES ";${data_name};")
set(data_file
${CMAKE_CURRENT_BINARY_DIR}/suites/test_suite_${data_name}.data)
set(dependency test_suite_bignum_generated_data)
elseif(";${psa_generated_data_names};" MATCHES ";${data_name};")
set(data_file
${CMAKE_CURRENT_BINARY_DIR}/suites/test_suite_${data_name}.data)
set(dependency test_suite_psa_generated_data)
else()
set(data_file
${CMAKE_CURRENT_SOURCE_DIR}/suites/test_suite_${data_name}.data)
set(dependency test_suite_bignum_generated_data test_suite_psa_generated_data)
endif()

add_custom_command(
Expand Down Expand Up @@ -129,7 +177,7 @@ function(add_test_suite suite_name)
)

add_executable(test_suite_${data_name} test_suite_${data_name}.c $<TARGET_OBJECTS:mbedtls_test>)
add_dependencies(test_suite_${data_name} test_suite_generated_data)
add_dependencies(test_suite_${data_name} ${dependency})
target_link_libraries(test_suite_${data_name} ${libs})
# Include test-specific header files from ./include and private header
# files (used by some invasive tests) from ../library. Public header
Expand Down
Loading