Re: [PATCH 18/24] selftests/resctrl: Introduce generalized test framework

[Reinette Chatre]

Hi Ilpo,

On 10/24/2023 2:26 AM, Ilpo Järvinen wrote:
...
> diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
> index ec6efd36f60a..e017adf1390d 100644
> --- a/tools/testing/selftests/resctrl/resctrl.h
> +++ b/tools/testing/selftests/resctrl/resctrl.h
> @@ -37,6 +37,8 @@
>  
>  #define DEFAULT_SPAN		(250 * MB)
>  
> +#define ARRAY_SIZE(arr)		(sizeof(arr) / sizeof((arr)[0]))
> +

Shuah worked hard to remove all these copies within kselftest. Please
use the one in kselftest.h instead.

>  #define PARENT_EXIT(err_msg)			\
>  	do {					\
>  		perror(err_msg);		\


...

> @@ -233,25 +183,26 @@ int main(int argc, char **argv)
>  		case 't':
>  			token = strtok(optarg, ",");
>  
> -			mbm_test = false;
> -			mba_test = false;
> -			cmt_test = false;
> -			cat_test = false;
> +			if (!test_param_seen) {
> +				for (i = 0; i < ARRAY_SIZE(resctrl_tests); i++)
> +					resctrl_tests[i]->disabled = true;
> +				tests = 0;
> +				test_param_seen = true;
> +			}
>  			while (token) {
> -				if (!strncmp(token, MBM_STR, sizeof(MBM_STR))) {
> -					mbm_test = true;
> -					tests++;
> -				} else if (!strncmp(token, MBA_STR, sizeof(MBA_STR))) {
> -					mba_test = true;
> -					tests++;
> -				} else if (!strncmp(token, CMT_STR, sizeof(CMT_STR))) {
> -					cmt_test = true;
> -					tests++;
> -				} else if (!strncmp(token, CAT_STR, sizeof(CAT_STR))) {
> -					cat_test = true;
> -					tests++;
> -				} else {
> -					printf("invalid argument\n");
> +				bool found = false;
> +
> +				for (i = 0; i < ARRAY_SIZE(resctrl_tests); i++) {
> +					if (!strcasecmp(token, resctrl_tests[i]->name)) {
> +						if (resctrl_tests[i]->disabled)
> +							tests++;
> +						resctrl_tests[i]->disabled = false;
> +						found = true;
> +					}
> +				}

Could providing multiple "-t" result in the test count not
matching the number of tests run?

> +
> +				if (!found) {
> +					printf("invalid test: %s\n", token);
>  
>  					return -1;
>  				}

A great improvement, thanks.

Reinette