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];.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];.desc = shell_test__description(bf, sizeof(bf), path, ent->d_name),
.desc = file->descif (!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