Re: [dpdk-dev] [PATCH] test/crypto: refactor docsis to show hidden cases

["Coyle, David" <david.coyle@intel.com>]

Hi Rebecca, see below


> -----Original Message-----
> From: Troy, Rebecca <rebecca.troy@intel.com>
> 
> In the current implementation, the docsis test cases are running and being
> reported as one test, despite the fact that multiple test cases are hidden
> inside i.e. "test_DOCSIS_PROTO_all" runs
> 52 test cases. Each docsis test case should be reported individually instead.

[DC] Should make "docsis" all uppercase in the commit message - "DOCSIS"

> 
> This commit achieves this by removing the use of the
> test_DOCSIS_PROTO_all function and statically listing the test cases to run
> when building the test suite, which are then reported to the user by
> description.
> 
> Signed-off-by: Rebecca Troy <rebecca.troy@intel.com>
> ---
>  app/test/test_cryptodev.c                     | 265 +++++++-----------
>  ...t_cryptodev_security_docsis_test_vectors.h | 159 +++++++++--
>  2 files changed, 241 insertions(+), 183 deletions(-)
> 
> 

<snip>

> +
>  static struct unit_test_suite docsis_proto_testsuite  = {
>  	.suite_name = "Docsis Proto Unit Test Suite",

[DC] Outside the specific changes of this patch, but "Docsis" should be all uppercase in the
suite name too - "DOCSIS"... could take this opportunity to fix up this minor one

>  	.setup = docsis_proto_testsuite_setup,
>  	.unit_test_cases = {
> -		TEST_CASE_ST(ut_setup_security, ut_teardown,
> -			test_DOCSIS_PROTO_all),
> +		/* Uplink */
> +		ADD_UPLINK_TESTCASE(docsis_test_case_1)
> +		ADD_UPLINK_TESTCASE(docsis_test_case_2)
> +		ADD_UPLINK_TESTCASE(docsis_test_case_3)
> +		ADD_UPLINK_TESTCASE(docsis_test_case_4)

<snip>

> 
> -struct docsis_test_data docsis_test_case_1 = {
> +const struct docsis_test_data docsis_test_case_1 = {
> +	.test_descr_uplink = {"AES-DOCSIS-BPI-128 and CRC Verify (24-byte "
> +			"frame, Small offset and runt block encryption)"},
> +	.test_descr_downlink = {"CRC Generate and AES-DOCSIS-BPI-128
> (24-byte "
> +			"frame, Small offset and runt block encryption)"},

[DC] This one is my fault when I supplied the descriptions, so apologies about this,
but all the uplink descriptions should say "decryption" instead of "encryption"

Also I think all the descriptions should say "Uplink" or "Downlink" at the start.
This can be inferred from the order of AES-DOCSIS-BPI and CRC in the description, but
when I ran the test cases, I still had to think are these Uplink or Downlink tests.
It would be much clearer if it's stated explicitly

Regards,
David