lkml.org 
[lkml]   [2022]   [Aug]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v5 01/14] perf test: Refactor shell tests allowing subdirs
From


On 8/6/22 09:37, Leo Yan wrote:
> On Thu, Jul 28, 2022 at 03:52:43PM +0100, carsten.haitzler@foss.arm.com wrote:
>
> [...]
>
>> +int list_script_max_width(void)
>> +{
>> + list_script_files(); /* Ensure we have scanned all scriptd */
>
> s/scriptd/scripts/

oops. fixed. v6 will come with that.

>> + return files_max_width;
>> +}
>
> [...]
>
>> struct shell_test {
>> const char *dir;
>> const char *file;
>> @@ -385,33 +302,17 @@ static int shell_test__run(struct test_suite *test, int subdir __maybe_unused)
>> static int run_shell_tests(int argc, const char *argv[], int i, int width,
>> struct intlist *skiplist)
>> {
>> - struct dirent **entlist;
>> - struct dirent *ent;
>> - int n_dirs, e;
>> - char path_dir[PATH_MAX];
>> - struct shell_test st = {
>> - .dir = shell_tests__dir(path_dir, sizeof(path_dir)),
>> - };
>> -
>> - if (st.dir == NULL)
>> - return -1;
>> + struct shell_test st;
>> + const struct script_file *files, *file;
>>
>> - n_dirs = scandir(st.dir, &entlist, NULL, alphasort);
>> - if (n_dirs == -1) {
>> - pr_err("failed to open shell test directory: %s\n",
>> - st.dir);
>> - return -1;
>> - }
>> -
>> - for_each_shell_test(entlist, n_dirs, st.dir, ent) {
>> + files = list_script_files();
>> + if (!files)
>> + return 0;
>> + for (file = files; file->dir; file++) {
>> int curr = i++;
>> - char desc[256];
>> struct test_case test_cases[] = {
>> {
>> - .desc = shell_test__description(desc,
>> - sizeof(desc),
>> - st.dir,
>> - ent->d_name),
>> + .desc = file->desc,
>> .run_case = shell_test__run,
>> },
>> { .name = NULL, }
>> @@ -421,12 +322,13 @@ static int run_shell_tests(int argc, const char *argv[], int i, int width,
>> .test_cases = test_cases,
>> .priv = &st,
>> };
>> + st.dir = file->dir;
>>
>> if (test_suite.desc == NULL ||
>> !perf_test__matches(test_suite.desc, curr, argc, argv))
>> continue;
>>
>> - st.file = ent->d_name;
>> + st.file = file->file;
>
> I am just wandering if we can remove "st" in this function, finally I
> found you are right, the "st" (struct shell_test) will be used in the
> function shell_test__run(), so let's keep as it is.
>
>> pr_info("%3d: %-*s:", i, width, test_suite.desc);
>>
>> if (intlist__find(skiplist, i)) {
>> @@ -436,10 +338,6 @@ static int run_shell_tests(int argc, const char *argv[], int i, int width,
>>
>> test_and_print(&test_suite, 0);
>> }
>> -
>> - for (e = 0; e < n_dirs; e++)
>> - zfree(&entlist[e]);
>> - free(entlist);
>> return 0;
>> }
>>
>> @@ -448,7 +346,7 @@ static int __cmd_test(int argc, const char *argv[], struct intlist *skiplist)
>> struct test_suite *t;
>> unsigned int j, k;
>> int i = 0;
>> - int width = shell_tests__max_desc_width();
>> + int width = list_script_max_width();
>>
>> for_each_test(j, k, t) {
>> int len = strlen(test_description(t, -1));
>> @@ -529,36 +427,22 @@ static int __cmd_test(int argc, const char *argv[], struct intlist *skiplist)
>>
>> static int perf_test__list_shell(int argc, const char **argv, int i)
>> {
>> - struct dirent **entlist;
>> - struct dirent *ent;
>> - int n_dirs, e;
>> - char path_dir[PATH_MAX];
>> - const char *path = shell_tests__dir(path_dir, sizeof(path_dir));
>> -
>> - if (path == NULL)
>> - return -1;
>> + const struct script_file *files, *file;
>>
>> - n_dirs = scandir(path, &entlist, NULL, alphasort);
>> - if (n_dirs == -1)
>> - return -1;
>> -
>> - for_each_shell_test(entlist, n_dirs, path, ent) {
>> + files = list_script_files();
>> + if (!files)
>> + return 0;
>> + for (file = files; file->dir; file++) {
>> int curr = i++;
>> - char bf[256];
>> struct test_suite t = {
>> - .desc = shell_test__description(bf, sizeof(bf), path, ent->d_name),
>> + .desc = file->desc
>> };
>>
>> if (!perf_test__matches(t.desc, curr, argc, argv))
>> continue;
>>
>> pr_info("%3d: %s\n", i, t.desc);
>> -
>> }
>> -
>> - for (e = 0; e < n_dirs; e++)
>> - zfree(&entlist[e]);
>> - free(entlist);
>> return 0;
>> }
>
> Except a minor typo, the patch looks good to me, it's a good
> refactoring and enhancement for shell script testing.
>
> I reviewed the change one by one line, at least I cannot find any logic
> error.
>
> With typo fixing:
>
> Reviewed-by: Leo Yan <leo.yan@linaro.org>
>
> I'd leave this patch for maintainers to review it. Just a caveat, given
> it's a big patch, as Carsten replied it's good that take the patch as a
> total new code for searching shell scripts, this would be easier for
> understanding the change.
>
> Thanks,
> Leo

\
 
 \ /
  Last update: 2022-08-10 10:39    [W:0.081 / U:0.648 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site