PSA tests: use a few common test macros #2
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gilles-peskine-arm
added
the
needs: review
Patater
reviewed
Dec 19, 2018
tests/suites/helpers.function
Outdated
| #endif | ||
|
|
||
| /* A compile-time constant with the value 0. If `const_expr` is not a | ||
| compile-time constant with a nonzero value, cause a compile-time error. */ |
There was a problem hiding this comment.
Style: begin continued comment lines with a *.
There was a problem hiding this comment.
Fixed in the amended commit.
tests/suites/helpers.function
Outdated
| #else | ||
| /* On platforms where we don't know how to implement this check, | ||
| * omit it. Oh well, a non-portable check is better than nothing. */ | ||
| #define STATIC_ASSERT_EXPR_IF_POSSIBLE( const_expr ) ( (void) 0 ) |
There was a problem hiding this comment.
Where is IS_ARRAY_NOT_POINTER defined for non GNUC platforms? Should STATIC_ASSERT_EXPR_IF_POSSIBLE instead be IS_ARRAY_NOT_POINTER? Where is STATIC_ASSERT_EXPR_IF_POSSIBLE used?
There was a problem hiding this comment.
Leftover from a slightly different design that I abandoned without committing. I have amended the corresponding commit.
gilles-peskine-arm
force-pushed
the
psa-test_macros
branch
from
de6c754 to
c08fc1d
Compare
Cause a compilation error on ARRAY_LENGTH(p) where p is a pointer as opposed to an array. This only works under GCC and compatible compilers such as Clang. On other compilers, ARRAY_LENGTH works but doesn't check the type of its argument.
It's potentially useful in all PSA test suites, of which there are now several.
TEST_EQUAL(expr1, expr2) is just TEST_ASSERT((expr1) == (expr2)) for now, but in the future I hope that it will print out the differing values.
This guarantees that they'll be indented as desired under most indentation rules.
Only whitespace changes in this commit.
This commit is the result of the following command, followed by
reindenting (but not wrapping lines):
perl -00 -i -pe 's/^( *)TEST_ASSERT\(([^;=]*)(?: |\n *)==\s*PSA_SUCCESS\s*\);$/${1}PSA_ASSERT($2 );/gm' tests/suites/test_suite_psa_*.function
This commit is the result of the following command, followed by
reindenting (but not wrapping lines):
perl -00 -i -pe 's/^( *)TEST_ASSERT\(([^;=]*)(?: |\n *)==([^;=]*)\);$/${1}TEST_EQUAL($2,$3);/gm' tests/suites/test_suite_psa_*.function
Change the way some lines are wrapped to cut at a more logical place. This commit mainly rewrites multi-line calls to TEST_EQUAL, and also a few calls to PSA_ASSERT.
Our code base doesn't even support 16-bit platforms, so those checks are always trivially true.
The test framework never passes NULL for a data_t* parameter, so testing them against NULL is clutter.
To allocate memory dynamically in a test, call ASSERT_ALLOC which takes care of calling calloc and of checking for NULL.
|
I've amended the commit "Add a safety check to ARRAY_LENGTH" to address the issues that @Patater pointed out. Previous history at https://github.com/gilles-peskine-arm/mbed-crypto/tree/psa-test_macros-1 |
|
CI failure is test_suite_timing on FreeBSD, a known issue. |
|
Release job: https://jenkins-internal.mbed.com/job/mbedtls-psa-release-new/201/ → PASS |
|
Other half of the release job: https://jenkins-internal.mbed.com/job/mbedtls-psa-release-new/202/ |
|
Basic build and test passed. Mbed OS testing went badly because the test is broke: not this PR's fault. |
NirSonnenschein
approved these changes
Dec 23, 2018
|
The test failure is the FreeBSD timing test, a known issue. |
Patater
pushed a commit
that referenced
this pull request
Jul 24, 2019
option name: LINK_WITH_TRUSTED_STORAGE default value: ON
gilles-peskine-arm
added a commit
to gilles-peskine-arm/mbed-crypto
that referenced
this pull request
Jul 31, 2019
Key creation and psa_get_key_attributes
Patater
added a commit
to Patater/mbed-crypto
that referenced
this pull request
Aug 16, 2019
* crypto/pr/212: (337 commits) Make TODO comments consistent Fix PSA tests Fix psa_generate_random for >1024 bytes Add tests to generate more random than MBEDTLS_CTR_DRBG_MAX_REQUEST Fix double free in psa_generate_key when psa_generate_random fails Fix copypasta in test data Avoid a lowercase letter in a macro name Correct some comments Fix PSA init/deinit in mbedtls_xxx tests when using PSA Make psa_calculate_key_bits return psa_key_bits_t Adjust secure element code to the new ITS interface More refactoring: consolidate attribute validation Fix policy validity check on key creation. Add test function for import with a bad policy Test key creation with an invalid type (0 and nonzero) Remove "allocated" flag from key slots Take advantage of psa_core_key_attributes_t internally ARMmbed#2 Store the key size in the slot in memory Take advantage of psa_core_key_attributes_t internally: key loading Switch storage functions over to psa_core_key_attributes_t ...
Patater
added a commit
that referenced
this pull request
Aug 16, 2019
* crypto/pr/212: (337 commits) Make TODO comments consistent Fix PSA tests Fix psa_generate_random for >1024 bytes Add tests to generate more random than MBEDTLS_CTR_DRBG_MAX_REQUEST Fix double free in psa_generate_key when psa_generate_random fails Fix copypasta in test data Avoid a lowercase letter in a macro name Correct some comments Fix PSA init/deinit in mbedtls_xxx tests when using PSA Make psa_calculate_key_bits return psa_key_bits_t Adjust secure element code to the new ITS interface More refactoring: consolidate attribute validation Fix policy validity check on key creation. Add test function for import with a bad policy Test key creation with an invalid type (0 and nonzero) Remove "allocated" flag from key slots Take advantage of psa_core_key_attributes_t internally #2 Store the key size in the slot in memory Take advantage of psa_core_key_attributes_t internally: key loading Switch storage functions over to psa_core_key_attributes_t ...
gilles-peskine-arm
added a commit
to gilles-peskine-arm/mbed-crypto
that referenced
this pull request
Sep 19, 2019
Make check-test-cases.py pass.
Prior to this commit, there were many repeated test descriptions, but
none with the same test data and dependencies and comments, as checked
with the following command:
for x in tests/suites/*.data; do perl -00 -ne 'warn "$ARGV: $. = $seen{$_}\n" if $seen{$_}; $seen{$_}=$.' $x; done
Wherever a test suite contains multiple test cases with the exact same
description, add " [ARMmbed#1]", " [ARMmbed#2]", etc. to make the descriptions
unique. We don't currently use this particular arrangement of
punctuation, so all occurrences of " [#" were added by this script.
I used the following ad hoc code:
import sys
def fix_test_suite(data_file_name):
in_paragraph = False
total = {}
index = {}
lines = None
with open(data_file_name) as data_file:
lines = list(data_file.readlines())
for line in lines:
if line == '\n':
in_paragraph = False
continue
if line.startswith('#'):
continue
if not in_paragraph:
# This is a test case description line.
total[line] = total.get(line, 0) + 1
index[line] = 0
in_paragraph = True
with open(data_file_name, 'w') as data_file:
for line in lines:
if line in total and total[line] > 1:
index[line] += 1
line = '%s [#%d]\n' % (line[:-1], index[line])
data_file.write(line)
for data_file_name in sys.argv[1:]:
fix_test_suite(data_file_name)
gilles-peskine-arm
added a commit
to gilles-peskine-arm/mbed-crypto
that referenced
this pull request
Sep 20, 2019
Make check-test-cases.py pass.
Prior to this commit, there were many repeated test descriptions, but
none with the same test data and dependencies and comments, as checked
with the following command:
for x in tests/suites/*.data; do perl -00 -ne 'warn "$ARGV: $. = $seen{$_}\n" if $seen{$_}; $seen{$_}=$.' $x; done
Wherever a test suite contains multiple test cases with the exact same
description, add " [ARMmbed#1]", " [ARMmbed#2]", etc. to make the descriptions
unique. We don't currently use this particular arrangement of
punctuation, so all occurrences of " [#" were added by this script.
I used the following ad hoc code:
import sys
def fix_test_suite(data_file_name):
in_paragraph = False
total = {}
index = {}
lines = None
with open(data_file_name) as data_file:
lines = list(data_file.readlines())
for line in lines:
if line == '\n':
in_paragraph = False
continue
if line.startswith('#'):
continue
if not in_paragraph:
# This is a test case description line.
total[line] = total.get(line, 0) + 1
index[line] = 0
in_paragraph = True
with open(data_file_name, 'w') as data_file:
for line in lines:
if line in total and total[line] > 1:
index[line] += 1
line = '%s [#%d]\n' % (line[:-1], index[line])
data_file.write(line)
for data_file_name in sys.argv[1:]:
fix_test_suite(data_file_name)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Declare a few macros in
helpers.functionand use them in the PSA tests. I haven't touched non-PSA tests.TEST_EQUAL(slightly more readable thanTEST_ASSERT(a==b), but the real payoff will come later when we instrument it to print the values on failure)PSA_ASSERT(TEST_ASSERT(…==PSA_SUCCESS))ARRAY_LENGTH(with protection against usage on a pointer when compiling with GCC)MIN,MAXThere's some refactoring going on, so I recommend reviewing commit by commit.