From 1be34dafab0d74d578a60c54b39699695c7ef643 Mon Sep 17 00:00:00 2001 From: Chris Jones Date: Mon, 18 Jan 2021 17:56:40 +0000 Subject: [PATCH 1/6] Remove redundant `test_info` assignment `test_fail` automatically sets `test_info.result`. This commit removes a case where `test_info.result` was being manually set after `test_fail` was called. Signed-off-by: Chris Jones --- tests/suites/main_test.function | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/suites/main_test.function b/tests/suites/main_test.function index 98dab3ebb..97026c68b 100644 --- a/tests/suites/main_test.function +++ b/tests/suites/main_test.function @@ -182,7 +182,6 @@ void execute_function_ptr(TestWrapper_t fp, void **params) test_fail( location_record.failure_condition, location_record.line, location_record.file ); - test_info.result = TEST_RESULT_FAILED; } mbedtls_test_param_failed_reset_state( ); From 9634bb10d9d0c83ce8d76b63201c4be473a0845e Mon Sep 17 00:00:00 2001 From: Chris Jones Date: Wed, 20 Jan 2021 15:56:42 +0000 Subject: [PATCH 2/6] Move helper testing functions to tests/src/helpers.c Moves the functions `test_fail`, `test_set_step`, `test_skip` and the struct `test_info` from `tests/suites/helpers.function` to `tests/src/helpers.*`. This is done to open these functions up to the API where they can be used by other functions in the 'src' test infrastructure module. As the functions are now contained within the src folder of the testing infrastructure, the `mbedtls_` prefix has been added to the functions. Signed-off-by: Chris Jones --- tests/include/test/helpers.h | 32 ++++++++ tests/src/helpers.c | 29 +++++++ tests/suites/helpers.function | 75 +++---------------- tests/suites/main_test.function | 6 +- tests/suites/test_suite_asn1parse.function | 6 +- tests/suites/test_suite_asn1write.function | 2 +- tests/suites/test_suite_psa_crypto.function | 17 +++-- ...st_suite_psa_crypto_se_driver_hal.function | 10 +-- tests/suites/test_suite_ssl.function | 10 +-- 9 files changed, 98 insertions(+), 89 deletions(-) diff --git a/tests/include/test/helpers.h b/tests/include/test/helpers.h index 2c7b179ab..8d31048a7 100644 --- a/tests/include/test/helpers.h +++ b/tests/include/test/helpers.h @@ -49,9 +49,41 @@ #include #include +typedef enum +{ + TEST_RESULT_SUCCESS = 0, + TEST_RESULT_FAILED, + TEST_RESULT_SKIPPED +} test_result_t; + +typedef struct +{ + test_result_t result; + const char *test; + const char *filename; + int line_no; + unsigned long step; +} +test_info_t; +extern test_info_t test_info; + int mbedtls_test_platform_setup( void ); void mbedtls_test_platform_teardown( void ); +void mbedtls_test_fail( const char *test, int line_no, const char* filename ); + +/** Set the test step number for failure reports. + * + * Call this function to display "step NNN" in addition to the line number + * and file name if a test fails. Typically the "step number" is the index + * of a for loop but it can be whatever you want. + * + * \param step The step number to report. + */ +void mbedtls_test_set_step( unsigned long step ); + +void mbedtls_test_skip( const char *test, int line_no, const char* filename ); + /** * \brief This function decodes the hexadecimal representation of * data. diff --git a/tests/src/helpers.c b/tests/src/helpers.c index a18f1d4b8..a2f9c3f34 100644 --- a/tests/src/helpers.c +++ b/tests/src/helpers.c @@ -44,6 +44,8 @@ static param_failed_ctx_t param_failed_ctx; static mbedtls_platform_context platform_ctx; #endif +test_info_t test_info; + /*----------------------------------------------------------------------------*/ /* Helper Functions */ @@ -77,6 +79,33 @@ static int ascii2uc(const char c, unsigned char *uc) return( 0 ); } +void mbedtls_test_fail( const char *test, int line_no, const char* filename ) +{ + if( test_info.result == TEST_RESULT_FAILED ) + { + /* We've already recorded the test as having failed. Don't + * overwrite any previous information about the failure. */ + return; + } + test_info.result = TEST_RESULT_FAILED; + test_info.test = test; + test_info.line_no = line_no; + test_info.filename = filename; +} + +void mbedtls_test_set_step( unsigned long step ) +{ + test_info.step = step; +} + +void mbedtls_test_skip( const char *test, int line_no, const char* filename ) +{ + test_info.result = TEST_RESULT_SKIPPED; + test_info.test = test; + test_info.line_no = line_no; + test_info.filename = filename; +} + int mbedtls_test_unhexify( unsigned char *obuf, size_t obufmax, const char *ibuf, diff --git a/tests/suites/helpers.function b/tests/suites/helpers.function index 1dc672153..9762d414d 100644 --- a/tests/suites/helpers.function +++ b/tests/suites/helpers.function @@ -108,7 +108,7 @@ typedef struct data_tag do { \ if( ! (TEST) ) \ { \ - test_fail( #TEST, __LINE__, __FILE__ ); \ + mbedtls_test_fail( #TEST, __LINE__, __FILE__ ); \ goto exit; \ } \ } while( 0 ) @@ -201,13 +201,13 @@ typedef struct data_tag * * \param TEST The test expression to be tested. */ -#define TEST_ASSUME( TEST ) \ - do { \ - if( ! (TEST) ) \ - { \ - test_skip( #TEST, __LINE__, __FILE__ ); \ - goto exit; \ - } \ +#define TEST_ASSUME( TEST ) \ + do { \ + if( ! (TEST) ) \ + { \ + mbedtls_test_skip( #TEST, __LINE__, __FILE__ ); \ + goto exit; \ + } \ } while( 0 ) #if defined(MBEDTLS_CHECK_PARAMS) && !defined(MBEDTLS_PARAM_FAILED_ALT) @@ -237,7 +237,7 @@ typedef struct data_tag if( ( ( TEST ) != ( PARAM_ERR_VALUE ) ) || \ ( mbedtls_test_param_failed_check_expected_call( ) != 0 ) ) \ { \ - test_fail( #TEST, __LINE__, __FILE__ ); \ + mbedtls_test_fail( #TEST, __LINE__, __FILE__ ); \ goto exit; \ } \ mbedtls_test_param_failed_check_expected_call( ); \ @@ -270,7 +270,7 @@ typedef struct data_tag if( setjmp( mbedtls_test_param_failed_get_state_buf( ) ) == 0 ) \ { \ TEST; \ - test_fail( #TEST, __LINE__, __FILE__ ); \ + mbedtls_test_fail( #TEST, __LINE__, __FILE__ ); \ goto exit; \ } \ mbedtls_test_param_failed_reset_state( ); \ @@ -346,24 +346,6 @@ typedef struct data_tag /*----------------------------------------------------------------------------*/ /* Global variables */ -typedef enum -{ - TEST_RESULT_SUCCESS = 0, - TEST_RESULT_FAILED, - TEST_RESULT_SKIPPED -} test_result_t; - -typedef struct -{ - test_result_t result; - const char *test; - const char *filename; - int line_no; - unsigned long step; -} -test_info_t; -static test_info_t test_info; - #if defined(MBEDTLS_CHECK_PARAMS) jmp_buf jmp_tmp; #endif @@ -386,41 +368,6 @@ jmp_buf jmp_tmp; /*----------------------------------------------------------------------------*/ /* Helper Functions */ -/** Set the test step number for failure reports. - * - * Call this function to display "step NNN" in addition to the line number - * and file name if a test fails. Typically the "step number" is the index - * of a for loop but it can be whatever you want. - * - * \param step The step number to report. - */ -void test_set_step( unsigned long step ) -{ - test_info.step = step; -} - -void test_fail( const char *test, int line_no, const char* filename ) -{ - if( test_info.result == TEST_RESULT_FAILED ) - { - /* We've already recorded the test as having failed. Don't - * overwrite any previous information about the failure. */ - return; - } - test_info.result = TEST_RESULT_FAILED; - test_info.test = test; - test_info.line_no = line_no; - test_info.filename = filename; -} - -void test_skip( const char *test, int line_no, const char* filename ) -{ - test_info.result = TEST_RESULT_SKIPPED; - test_info.test = test; - test_info.line_no = line_no; - test_info.filename = filename; -} - #if defined(MBEDTLS_PSA_CRYPTO_C) /** Check that no PSA Crypto key slots are in use. * @@ -435,7 +382,7 @@ int test_fail_if_psa_leaking( int line_no, const char *filename ) return 0; else { - test_fail( msg, line_no, filename ); + mbedtls_test_fail( msg, line_no, filename ); return 1; } } diff --git a/tests/suites/main_test.function b/tests/suites/main_test.function index 97026c68b..57395aebb 100644 --- a/tests/suites/main_test.function +++ b/tests/suites/main_test.function @@ -179,9 +179,9 @@ void execute_function_ptr(TestWrapper_t fp, void **params) { /* Unexpected parameter validation error */ mbedtls_test_param_failed_get_location_record( &location_record ); - test_fail( location_record.failure_condition, - location_record.line, - location_record.file ); + mbedtls_test_fail( location_record.failure_condition, + location_record.line, + location_record.file ); } mbedtls_test_param_failed_reset_state( ); diff --git a/tests/suites/test_suite_asn1parse.function b/tests/suites/test_suite_asn1parse.function index 990f343a7..51cb3ca9c 100644 --- a/tests/suites/test_suite_asn1parse.function +++ b/tests/suites/test_suite_asn1parse.function @@ -130,7 +130,7 @@ int get_len_step( const data_t *input, size_t buffer_size, size_t parsed_length; int ret; - test_set_step( buffer_size ); + mbedtls_test_set_step( buffer_size ); /* Allocate a new buffer of exactly the length to parse each time. * This gives memory sanitizers a chance to catch buffer overreads. */ if( buffer_size == 0 ) @@ -198,7 +198,7 @@ static int traverse_callback( void *ctx, int tag, TEST_ASSERT( content > state->input_start ); offset = content - state->input_start; - test_set_step( offset ); + mbedtls_test_set_step( offset ); if( *rest == 0 ) return( RET_TRAVERSE_STOP ); @@ -252,7 +252,7 @@ void parse_prefixes( const data_t *input, */ for( buffer_size = 1; buffer_size <= input->len + 1; buffer_size++ ) { - test_set_step( buffer_size ); + mbedtls_test_set_step( buffer_size ); /* Allocate a new buffer of exactly the length to parse each time. * This gives memory sanitizers a chance to catch buffer overreads. */ ASSERT_ALLOC( buf, buffer_size ); diff --git a/tests/suites/test_suite_asn1write.function b/tests/suites/test_suite_asn1write.function index 21465c756..882473905 100644 --- a/tests/suites/test_suite_asn1write.function +++ b/tests/suites/test_suite_asn1write.function @@ -15,7 +15,7 @@ typedef struct int generic_write_start_step( generic_write_data_t *data ) { - test_set_step( data->size ); + mbedtls_test_set_step( data->size ); ASSERT_ALLOC( data->output, data->size == 0 ? 1 : data->size ); data->end = data->output + data->size; data->p = data->end; diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index d486dd19c..a45d7e0b1 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -537,7 +537,8 @@ static int exercise_signature_key( mbedtls_svc_key_id_t key, hash_alg = KNOWN_SUPPORTED_HASH_ALG; alg ^= PSA_ALG_ANY_HASH ^ hash_alg; #else - test_fail( "No hash algorithm for hash-and-sign testing", __LINE__, __FILE__ ); + mbedtls_test_fail( "No hash algorithm for hash-and-sign testing", + __LINE__, __FILE__ ); return( 1 ); #endif } @@ -997,7 +998,7 @@ static int exported_key_sanity_check( psa_key_type_t type, size_t bits, mbedtls_snprintf( message, sizeof( message ), "No sanity check for public key type=0x%08lx", (unsigned long) type ); - test_fail( message, __LINE__, __FILE__ ); + mbedtls_test_fail( message, __LINE__, __FILE__ ); (void) p; (void) end; return( 0 ); @@ -1111,8 +1112,8 @@ exit: * asymmetric, also check \p psa_export_public_key. * * If the key fails the tests, this function calls the test framework's - * `test_fail` function and returns false. Otherwise this function returns - * true. Therefore it should be used as follows: + * `mbedtls_test_fail` function and returns false. Otherwise this function + * returns true. Therefore it should be used as follows: * ``` * if( ! exercise_key( ... ) ) goto exit; * ``` @@ -1158,7 +1159,7 @@ static int exercise_key( mbedtls_svc_key_id_t key, mbedtls_snprintf( message, sizeof( message ), "No code to exercise alg=0x%08lx", (unsigned long) alg ); - test_fail( message, __LINE__, __FILE__ ); + mbedtls_test_fail( message, __LINE__, __FILE__ ); ok = 0; } @@ -2672,7 +2673,7 @@ void hash_compute_compare( int alg_arg, data_t *input, /* Compare with corrupted value */ for( i = 0; i < output_length; i++ ) { - test_set_step( i ); + mbedtls_test_set_step( i ); output[i] ^= 1; TEST_EQUAL( psa_hash_compare( alg, input->x, input->len, output, output_length ), @@ -3147,7 +3148,7 @@ void mac_sign( int key_type_arg, ( output_size >= expected_mac->len ? PSA_SUCCESS : PSA_ERROR_BUFFER_TOO_SMALL ); - test_set_step( output_size ); + mbedtls_test_set_step( output_size ); ASSERT_ALLOC( actual_mac, output_size ); /* Calculate the MAC. */ @@ -3233,7 +3234,7 @@ void mac_verify( int key_type_arg, /* Test changing one byte. */ for( size_t i = 0; i < expected_mac->len; i++ ) { - test_set_step( i ); + mbedtls_test_set_step( i ); perturbed_mac[i] ^= 1; PSA_ASSERT( psa_mac_verify_setup( &operation, key, alg ) ); PSA_ASSERT( psa_mac_update( &operation, diff --git a/tests/suites/test_suite_psa_crypto_se_driver_hal.function b/tests/suites/test_suite_psa_crypto_se_driver_hal.function index d623221a5..59bbb6e30 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.function +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.function @@ -45,7 +45,7 @@ do { \ if( ! (TEST) ) \ { \ - test_fail( #TEST, __LINE__, __FILE__ ); \ + mbedtls_test_fail( #TEST, __LINE__, __FILE__ ); \ return( PSA_ERROR_DETECTED_BY_DRIVER ); \ } \ } while( 0 ) @@ -61,7 +61,7 @@ do { \ if( ! (TEST) ) \ { \ - test_fail( #TEST, __LINE__, __FILE__ ); \ + mbedtls_test_fail( #TEST, __LINE__, __FILE__ ); \ status = PSA_ERROR_DETECTED_BY_DRIVER; \ goto exit; \ } \ @@ -72,10 +72,10 @@ * Run the code \p expr. If this returns \p expected_status, * do nothing. If this returns #PSA_ERROR_DETECTED_BY_DRIVER, * jump directly to the `exit` label. If this returns any other - * status, call test_fail() then jump to `exit`. + * status, call mbedtls_test_fail() then jump to `exit`. * * The special case for #PSA_ERROR_DETECTED_BY_DRIVER is because in this - * case, the test driver code is expected to have called test_fail() + * case, the test driver code is expected to have called mbedtls_test_fail() * already, so we make sure not to overwrite the failure information. */ #define PSA_ASSERT_VIA_DRIVER( expr, expected_status ) \ @@ -85,7 +85,7 @@ goto exit; \ if( PSA_ASSERT_VIA_DRIVER_status != ( expected_status ) ) \ { \ - test_fail( #expr, __LINE__, __FILE__ ); \ + mbedtls_test_fail( #expr, __LINE__, __FILE__ ); \ goto exit; \ } \ } while( 0 ) diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index f377ffa99..568e66bb4 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -3609,7 +3609,7 @@ void ssl_decrypt_non_etm_cbc( int cipher_type, int hash_id, int trunc_hmac, */ for( i = block_size; i < buflen; i++ ) { - test_set_step( i ); + mbedtls_test_set_step( i ); /* Restore correct pre-encryption record */ rec = rec_save; @@ -3646,7 +3646,7 @@ void ssl_decrypt_non_etm_cbc( int cipher_type, int hash_id, int trunc_hmac, */ for( i = padlen; i <= pad_max_len; i++ ) { - test_set_step( i ); + mbedtls_test_set_step( i ); /* Restore correct pre-encryption record */ rec = rec_save; @@ -4466,7 +4466,7 @@ void ssl_cf_hmac( int hash ) */ for( max_in_len = 0; max_in_len <= 255 + block_size; max_in_len++ ) { - test_set_step( max_in_len * 10000 ); + mbedtls_test_set_step( max_in_len * 10000 ); /* Use allocated in buffer to catch overreads */ ASSERT_ALLOC( data, max_in_len ); @@ -4474,7 +4474,7 @@ void ssl_cf_hmac( int hash ) min_in_len = max_in_len > 255 ? max_in_len - 255 : 0; for( in_len = min_in_len; in_len <= max_in_len; in_len++ ) { - test_set_step( max_in_len * 10000 + in_len ); + mbedtls_test_set_step( max_in_len * 10000 + in_len ); /* Set up dummy data and add_data */ rec_num++; @@ -4531,7 +4531,7 @@ void ssl_cf_memcpy_offset( int offset_min, int offset_max, int len ) for( secret = offset_min; secret <= (size_t) offset_max; secret++ ) { - test_set_step( (int) secret ); + mbedtls_test_set_step( (int) secret ); TEST_CF_SECRET( &secret, sizeof( secret ) ); mbedtls_ssl_cf_memcpy_offset( dst, src, secret, From e60e2aeb7462a05ee2b0398e5c66cf2c2bc29776 Mon Sep 17 00:00:00 2001 From: Chris Jones Date: Wed, 20 Jan 2021 17:51:47 +0000 Subject: [PATCH 3/6] Add `mbedtls_` prefix to all public names in `helpers.h` Adds the `mbedtls_` prefix to `test_result_t` and `test_info` and updates any references to them. This is to follow the naming convention as these are now declared in a public namespace. Signed-off-by: Chris Jones --- tests/include/test/helpers.h | 14 ++++----- tests/src/helpers.c | 22 +++++++------- tests/suites/host_test.function | 34 ++++++++++++---------- tests/suites/target_test.function | 6 ++-- tests/suites/test_suite_asn1parse.function | 2 +- 5 files changed, 40 insertions(+), 38 deletions(-) diff --git a/tests/include/test/helpers.h b/tests/include/test/helpers.h index 8d31048a7..2ca85d479 100644 --- a/tests/include/test/helpers.h +++ b/tests/include/test/helpers.h @@ -51,21 +51,21 @@ typedef enum { - TEST_RESULT_SUCCESS = 0, - TEST_RESULT_FAILED, - TEST_RESULT_SKIPPED -} test_result_t; + MBEDTLS_TEST_RESULT_SUCCESS = 0, + MBEDTLS_TEST_RESULT_FAILED, + MBEDTLS_TEST_RESULT_SKIPPED +} mbedtls_test_result_t; typedef struct { - test_result_t result; + mbedtls_test_result_t result; const char *test; const char *filename; int line_no; unsigned long step; } -test_info_t; -extern test_info_t test_info; +mbedtls_test_info_t; +extern mbedtls_test_info_t mbedtls_test_info; int mbedtls_test_platform_setup( void ); void mbedtls_test_platform_teardown( void ); diff --git a/tests/src/helpers.c b/tests/src/helpers.c index a2f9c3f34..9bfd7e047 100644 --- a/tests/src/helpers.c +++ b/tests/src/helpers.c @@ -44,7 +44,7 @@ static param_failed_ctx_t param_failed_ctx; static mbedtls_platform_context platform_ctx; #endif -test_info_t test_info; +mbedtls_test_info_t mbedtls_test_info; /*----------------------------------------------------------------------------*/ /* Helper Functions */ @@ -81,29 +81,29 @@ static int ascii2uc(const char c, unsigned char *uc) void mbedtls_test_fail( const char *test, int line_no, const char* filename ) { - if( test_info.result == TEST_RESULT_FAILED ) + if( mbedtls_test_info.result == MBEDTLS_TEST_RESULT_FAILED ) { /* We've already recorded the test as having failed. Don't * overwrite any previous information about the failure. */ return; } - test_info.result = TEST_RESULT_FAILED; - test_info.test = test; - test_info.line_no = line_no; - test_info.filename = filename; + mbedtls_test_info.result = MBEDTLS_TEST_RESULT_FAILED; + mbedtls_test_info.test = test; + mbedtls_test_info.line_no = line_no; + mbedtls_test_info.filename = filename; } void mbedtls_test_set_step( unsigned long step ) { - test_info.step = step; + mbedtls_test_info.step = step; } void mbedtls_test_skip( const char *test, int line_no, const char* filename ) { - test_info.result = TEST_RESULT_SKIPPED; - test_info.test = test; - test_info.line_no = line_no; - test_info.filename = filename; + mbedtls_test_info.result = MBEDTLS_TEST_RESULT_SKIPPED; + mbedtls_test_info.test = test; + mbedtls_test_info.line_no = line_no; + mbedtls_test_info.filename = filename; } int mbedtls_test_unhexify( unsigned char *obuf, diff --git a/tests/suites/host_test.function b/tests/suites/host_test.function index 872a3a43a..b5581b551 100644 --- a/tests/suites/host_test.function +++ b/tests/suites/host_test.function @@ -428,15 +428,15 @@ static void write_outcome_entry( FILE *outcome_file, * \param unmet_dependencies The array of unmet dependencies. * \param missing_unmet_dependencies Non-zero if there was a problem tracking * all unmet dependencies, 0 otherwise. - * \param ret The test dispatch status (DISPATCH_xxx). - * \param test_info A pointer to the test info structure. + * \param ret The test dispatch status (DISPATCH_xxx). + * \param mbedtls_test_info A pointer to the test info structure. */ static void write_outcome_result( FILE *outcome_file, size_t unmet_dep_count, int unmet_dependencies[], int missing_unmet_dependencies, int ret, - const test_info_t *info ) + const mbedtls_test_info_t *info ) { if( outcome_file == NULL ) return; @@ -462,10 +462,10 @@ static void write_outcome_result( FILE *outcome_file, } switch( info->result ) { - case TEST_RESULT_SUCCESS: + case MBEDTLS_TEST_RESULT_SUCCESS: mbedtls_fprintf( outcome_file, "PASS;" ); break; - case TEST_RESULT_SKIPPED: + case MBEDTLS_TEST_RESULT_SKIPPED: mbedtls_fprintf( outcome_file, "SKIP;Runtime skip" ); break; default: @@ -601,7 +601,7 @@ int execute_tests( int argc , const char ** argv ) } /* Initialize the struct that holds information about the last test */ - memset( &test_info, 0, sizeof( test_info ) ); + memset( &mbedtls_test_info, 0, sizeof( mbedtls_test_info ) ); /* Now begin to execute the tests in the testfiles */ for ( testfile_index = 0; @@ -638,7 +638,8 @@ int execute_tests( int argc , const char ** argv ) if( ( ret = get_line( file, buf, sizeof(buf) ) ) != 0 ) break; mbedtls_fprintf( stdout, "%s%.66s", - test_info.result == TEST_RESULT_FAILED ? "\n" : "", buf ); + mbedtls_test_info.result == MBEDTLS_TEST_RESULT_FAILED ? + "\n" : "", buf ); mbedtls_fprintf( stdout, " " ); for( i = strlen( buf ) + 1; i < 67; i++ ) mbedtls_fprintf( stdout, "." ); @@ -682,8 +683,8 @@ int execute_tests( int argc , const char ** argv ) // If there are no unmet dependencies execute the test if( unmet_dep_count == 0 ) { - test_info.result = TEST_RESULT_SUCCESS; - test_info.step = (unsigned long)( -1 ); + mbedtls_test_info.result = MBEDTLS_TEST_RESULT_SUCCESS; + mbedtls_test_info.step = (unsigned long)( -1 ); #if defined(__unix__) || (defined(__APPLE__) && defined(__MACH__)) /* Suppress all output from the library unless we're verbose @@ -723,7 +724,7 @@ int execute_tests( int argc , const char ** argv ) write_outcome_result( outcome_file, unmet_dep_count, unmet_dependencies, missing_unmet_dependencies, - ret, &test_info ); + ret, &mbedtls_test_info ); if( unmet_dep_count > 0 || ret == DISPATCH_UNSUPPORTED_SUITE ) { total_skipped++; @@ -753,11 +754,11 @@ int execute_tests( int argc , const char ** argv ) } else if( ret == DISPATCH_TEST_SUCCESS ) { - if( test_info.result == TEST_RESULT_SUCCESS ) + if( mbedtls_test_info.result == MBEDTLS_TEST_RESULT_SUCCESS ) { mbedtls_fprintf( stdout, "PASS\n" ); } - else if( test_info.result == TEST_RESULT_SKIPPED ) + else if( mbedtls_test_info.result == MBEDTLS_TEST_RESULT_SKIPPED ) { mbedtls_fprintf( stdout, "----\n" ); total_skipped++; @@ -767,14 +768,15 @@ int execute_tests( int argc , const char ** argv ) total_errors++; mbedtls_fprintf( stdout, "FAILED\n" ); mbedtls_fprintf( stdout, " %s\n at ", - test_info.test ); - if( test_info.step != (unsigned long)( -1 ) ) + mbedtls_test_info.test ); + if( mbedtls_test_info.step != (unsigned long)( -1 ) ) { mbedtls_fprintf( stdout, "step %lu, ", - test_info.step ); + mbedtls_test_info.step ); } mbedtls_fprintf( stdout, "line %d, %s", - test_info.line_no, test_info.filename ); + mbedtls_test_info.line_no, + mbedtls_test_info.filename ); } fflush( stdout ); } diff --git a/tests/suites/target_test.function b/tests/suites/target_test.function index 8354b968c..7866dcfb3 100644 --- a/tests/suites/target_test.function +++ b/tests/suites/target_test.function @@ -384,8 +384,8 @@ int execute_tests( int args, const char ** argv ) while ( 1 ) { ret = 0; - test_info.result = TEST_RESULT_SUCCESS; - test_info.step = (unsigned long)( -1 ); + mbedtls_test_info.result = MBEDTLS_TEST_RESULT_SUCCESS; + mbedtls_test_info.step = (unsigned long)( -1 ); data_len = 0; data = receive_data( &data_len ); @@ -443,7 +443,7 @@ int execute_tests( int args, const char ** argv ) if ( ret ) send_failure( ret ); else - send_status( test_info.result ); + send_status( mbedtls_test_info.result ); } return( 0 ); } diff --git a/tests/suites/test_suite_asn1parse.function b/tests/suites/test_suite_asn1parse.function index 51cb3ca9c..75c3fd40c 100644 --- a/tests/suites/test_suite_asn1parse.function +++ b/tests/suites/test_suite_asn1parse.function @@ -614,7 +614,7 @@ void get_sequence_of( const data_t *input, int tag, cur = &head; while( *rest ) { - ++test_info.step; + ++mbedtls_test_info.step; TEST_ASSERT( cur != NULL ); TEST_EQUAL( cur->buf.tag, tag ); n = strtoul( rest, (char **) &rest, 0 ); From a5ab765832dce4a4d0747febf7a3307210b8d6ed Mon Sep 17 00:00:00 2001 From: Chris Jones Date: Tue, 2 Feb 2021 16:20:45 +0000 Subject: [PATCH 4/6] Remove direct writing to `test_info` from `*.function` Add a new function `mbedtls_test_info_reset()` to remove direct writes to `mbedtls_test_info`. This change still allows values to be read directly however all writes are now done inside of `helpers.c`. Also slightly reordered code to make it easier to read. Signed-off-by: Chris Jones --- tests/include/test/helpers.h | 3 ++- tests/src/helpers.c | 19 ++++++++++++++----- tests/suites/host_test.function | 5 ++--- tests/suites/target_test.function | 3 +-- tests/suites/test_suite_asn1parse.function | 2 +- 5 files changed, 20 insertions(+), 12 deletions(-) diff --git a/tests/include/test/helpers.h b/tests/include/test/helpers.h index 2ca85d479..258c6bae4 100644 --- a/tests/include/test/helpers.h +++ b/tests/include/test/helpers.h @@ -71,6 +71,7 @@ int mbedtls_test_platform_setup( void ); void mbedtls_test_platform_teardown( void ); void mbedtls_test_fail( const char *test, int line_no, const char* filename ); +void mbedtls_test_skip( const char *test, int line_no, const char* filename ); /** Set the test step number for failure reports. * @@ -82,7 +83,7 @@ void mbedtls_test_fail( const char *test, int line_no, const char* filename ); */ void mbedtls_test_set_step( unsigned long step ); -void mbedtls_test_skip( const char *test, int line_no, const char* filename ); +void mbedtls_test_info_reset( void ); /** * \brief This function decodes the hexadecimal representation of diff --git a/tests/src/helpers.c b/tests/src/helpers.c index 9bfd7e047..e323275e5 100644 --- a/tests/src/helpers.c +++ b/tests/src/helpers.c @@ -93,11 +93,6 @@ void mbedtls_test_fail( const char *test, int line_no, const char* filename ) mbedtls_test_info.filename = filename; } -void mbedtls_test_set_step( unsigned long step ) -{ - mbedtls_test_info.step = step; -} - void mbedtls_test_skip( const char *test, int line_no, const char* filename ) { mbedtls_test_info.result = MBEDTLS_TEST_RESULT_SKIPPED; @@ -106,6 +101,20 @@ void mbedtls_test_skip( const char *test, int line_no, const char* filename ) mbedtls_test_info.filename = filename; } +void mbedtls_test_set_step( unsigned long step ) +{ + mbedtls_test_info.step = step; +} + +void mbedtls_test_info_reset( void ) +{ + mbedtls_test_info.result = MBEDTLS_TEST_RESULT_SUCCESS; + mbedtls_test_info.step = (unsigned long)( -1 ); + mbedtls_test_info.test = 0; + mbedtls_test_info.line_no = 0; + mbedtls_test_info.filename = 0; +} + int mbedtls_test_unhexify( unsigned char *obuf, size_t obufmax, const char *ibuf, diff --git a/tests/suites/host_test.function b/tests/suites/host_test.function index b5581b551..f3454086c 100644 --- a/tests/suites/host_test.function +++ b/tests/suites/host_test.function @@ -601,7 +601,7 @@ int execute_tests( int argc , const char ** argv ) } /* Initialize the struct that holds information about the last test */ - memset( &mbedtls_test_info, 0, sizeof( mbedtls_test_info ) ); + mbedtls_test_info_reset(); /* Now begin to execute the tests in the testfiles */ for ( testfile_index = 0; @@ -683,8 +683,7 @@ int execute_tests( int argc , const char ** argv ) // If there are no unmet dependencies execute the test if( unmet_dep_count == 0 ) { - mbedtls_test_info.result = MBEDTLS_TEST_RESULT_SUCCESS; - mbedtls_test_info.step = (unsigned long)( -1 ); + mbedtls_test_info_reset(); #if defined(__unix__) || (defined(__APPLE__) && defined(__MACH__)) /* Suppress all output from the library unless we're verbose diff --git a/tests/suites/target_test.function b/tests/suites/target_test.function index 7866dcfb3..8afe70eaf 100644 --- a/tests/suites/target_test.function +++ b/tests/suites/target_test.function @@ -384,8 +384,7 @@ int execute_tests( int args, const char ** argv ) while ( 1 ) { ret = 0; - mbedtls_test_info.result = MBEDTLS_TEST_RESULT_SUCCESS; - mbedtls_test_info.step = (unsigned long)( -1 ); + mbedtls_test_info_reset(); data_len = 0; data = receive_data( &data_len ); diff --git a/tests/suites/test_suite_asn1parse.function b/tests/suites/test_suite_asn1parse.function index 75c3fd40c..21b3ab610 100644 --- a/tests/suites/test_suite_asn1parse.function +++ b/tests/suites/test_suite_asn1parse.function @@ -614,7 +614,7 @@ void get_sequence_of( const data_t *input, int tag, cur = &head; while( *rest ) { - ++mbedtls_test_info.step; + mbedtls_test_set_step( mbedtls_test_info.step + 1 ); TEST_ASSERT( cur != NULL ); TEST_EQUAL( cur->buf.tag, tag ); n = strtoul( rest, (char **) &rest, 0 ); From 567e0ad8f1bcddab312d521076e944e970816b90 Mon Sep 17 00:00:00 2001 From: Chris Jones Date: Wed, 3 Feb 2021 12:07:01 +0000 Subject: [PATCH 5/6] Add documentation and minor style changes Add doxygen style documentation to `mbedtls_test_fail`, `mbedtls_test_skip`, `mbedtls_test_set_step` and `mbedtls_test_info_reset`. This should make it easier to understand how the test infrastructure is used. Also make some minor style changes to meet the coding standards and make it more obvious that `mbedtls_test_info.step` was being incremented. Signed-off-by: Chris Jones --- tests/include/test/helpers.h | 31 +++++++++++++++++++--- tests/suites/host_test.function | 4 +-- tests/suites/target_test.function | 2 +- tests/suites/test_suite_asn1parse.function | 4 ++- 4 files changed, 33 insertions(+), 8 deletions(-) diff --git a/tests/include/test/helpers.h b/tests/include/test/helpers.h index 258c6bae4..50d07fca6 100644 --- a/tests/include/test/helpers.h +++ b/tests/include/test/helpers.h @@ -70,19 +70,42 @@ extern mbedtls_test_info_t mbedtls_test_info; int mbedtls_test_platform_setup( void ); void mbedtls_test_platform_teardown( void ); +/** + * \brief Record the given, usually current, test as a failure. + * + * \param test Name of the test to fail. + * \param line_no Line number where the failure originated. + * \param filename Filename where the failure originated. + */ void mbedtls_test_fail( const char *test, int line_no, const char* filename ); + +/** + * \brief Record the given, usually current, test as skipped. + * + * \param test Name of the test to skip. + * \param line_no Line number where the test skipped. + * \param filename Filename where the test skipped. + */ void mbedtls_test_skip( const char *test, int line_no, const char* filename ); -/** Set the test step number for failure reports. +/** + * \brief Set the test step number for failure reports. * - * Call this function to display "step NNN" in addition to the line number - * and file name if a test fails. Typically the "step number" is the index - * of a for loop but it can be whatever you want. + * \note Call this function to display "step NNN" in addition to the + * line number and file name if a test fails. Typically the "step + * number" is the index of a for loop but it can be whatever you + * want. * * \param step The step number to report. */ void mbedtls_test_set_step( unsigned long step ); +/** + * \brief Reset mbedtls_test_info to a ready/starting state. + * + * \note Clears the test, line_no, filename, step and result from any + * previously stored values and initialises them ready to be used. + */ void mbedtls_test_info_reset( void ); /** diff --git a/tests/suites/host_test.function b/tests/suites/host_test.function index f3454086c..3138c3365 100644 --- a/tests/suites/host_test.function +++ b/tests/suites/host_test.function @@ -601,7 +601,7 @@ int execute_tests( int argc , const char ** argv ) } /* Initialize the struct that holds information about the last test */ - mbedtls_test_info_reset(); + mbedtls_test_info_reset( ); /* Now begin to execute the tests in the testfiles */ for ( testfile_index = 0; @@ -683,7 +683,7 @@ int execute_tests( int argc , const char ** argv ) // If there are no unmet dependencies execute the test if( unmet_dep_count == 0 ) { - mbedtls_test_info_reset(); + mbedtls_test_info_reset( ); #if defined(__unix__) || (defined(__APPLE__) && defined(__MACH__)) /* Suppress all output from the library unless we're verbose diff --git a/tests/suites/target_test.function b/tests/suites/target_test.function index 8afe70eaf..637a79d5e 100644 --- a/tests/suites/target_test.function +++ b/tests/suites/target_test.function @@ -384,7 +384,7 @@ int execute_tests( int args, const char ** argv ) while ( 1 ) { ret = 0; - mbedtls_test_info_reset(); + mbedtls_test_info_reset( ); data_len = 0; data = receive_data( &data_len ); diff --git a/tests/suites/test_suite_asn1parse.function b/tests/suites/test_suite_asn1parse.function index 21b3ab610..47a43407d 100644 --- a/tests/suites/test_suite_asn1parse.function +++ b/tests/suites/test_suite_asn1parse.function @@ -594,6 +594,7 @@ void get_sequence_of( const data_t *input, int tag, unsigned char *p = input->x; const char *rest = description; unsigned long n; + unsigned int step = 0; TEST_EQUAL( mbedtls_asn1_get_sequence_of( &p, input->x + input->len, &head, tag ), @@ -614,7 +615,7 @@ void get_sequence_of( const data_t *input, int tag, cur = &head; while( *rest ) { - mbedtls_test_set_step( mbedtls_test_info.step + 1 ); + mbedtls_test_set_step( step ); TEST_ASSERT( cur != NULL ); TEST_EQUAL( cur->buf.tag, tag ); n = strtoul( rest, (char **) &rest, 0 ); @@ -625,6 +626,7 @@ void get_sequence_of( const data_t *input, int tag, if( *rest ) ++rest; cur = cur->next; + ++step; } TEST_ASSERT( cur == NULL ); } From 39ddb0a2e1cba382a1e4cd05a6180dd38efa541d Mon Sep 17 00:00:00 2001 From: Chris Jones Date: Wed, 3 Feb 2021 16:15:00 +0000 Subject: [PATCH 6/6] Improve test infrastructure documentation Clarify the function descriptions for several test infrastructure functions. Signed-off-by: Chris Jones --- tests/include/test/helpers.h | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/tests/include/test/helpers.h b/tests/include/test/helpers.h index 50d07fca6..59260d902 100644 --- a/tests/include/test/helpers.h +++ b/tests/include/test/helpers.h @@ -71,27 +71,40 @@ int mbedtls_test_platform_setup( void ); void mbedtls_test_platform_teardown( void ); /** - * \brief Record the given, usually current, test as a failure. + * \brief Record the current test case as a failure. * - * \param test Name of the test to fail. + * This function can be called directly however it is usually + * called via macros such as TEST_ASSERT, TEST_EQUAL, + * PSA_ASSERT, etc... + * + * \note If the test case was already marked as failed, calling + * `mbedtls_test_fail( )` again will not overwrite any + * previous information about the failure. + * + * \param test Description of the failure or assertion that failed. This + * MUST be a string literal. * \param line_no Line number where the failure originated. * \param filename Filename where the failure originated. */ void mbedtls_test_fail( const char *test, int line_no, const char* filename ); /** - * \brief Record the given, usually current, test as skipped. + * \brief Record the current test case as skipped. * - * \param test Name of the test to skip. - * \param line_no Line number where the test skipped. - * \param filename Filename where the test skipped. + * This function can be called directly however it is usually + * called via the TEST_ASSUME macro. + * + * \param test Description of the assumption that caused the test case to + * be skipped. This MUST be a string literal. + * \param line_no Line number where the test case was skipped. + * \param filename Filename where the test case was skipped. */ void mbedtls_test_skip( const char *test, int line_no, const char* filename ); /** * \brief Set the test step number for failure reports. * - * \note Call this function to display "step NNN" in addition to the + * Call this function to display "step NNN" in addition to the * line number and file name if a test fails. Typically the "step * number" is the index of a for loop but it can be whatever you * want. @@ -102,9 +115,6 @@ void mbedtls_test_set_step( unsigned long step ); /** * \brief Reset mbedtls_test_info to a ready/starting state. - * - * \note Clears the test, line_no, filename, step and result from any - * previously stored values and initialises them ready to be used. */ void mbedtls_test_info_reset( void );