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

["Troy, Rebecca" <rebecca.troy@intel.com>]

> -----Original Message-----
> From: Coyle, David <david.coyle@intel.com>
> Sent: Thursday 28 October 2021 11:34
> To: Troy, Rebecca <rebecca.troy@intel.com>; dev@dpdk.org
> Cc: Power, Ciara <ciara.power@intel.com>; Zhang, Roy Fan
> <roy.fan.zhang@intel.com>; Akhil Goyal <gakhil@marvell.com>; Doherty,
> Declan <declan.doherty@intel.com>
> Subject: RE: [PATCH] test/crypto: refactor docsis to show hidden cases
> 
> 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

Great, thanks for the feedback David! Will look at implementing these changes in a v2.

Thanks,
Rebecca.