Add ability to parse multiple files. Additionally add the
ability to parse all results in the KUnit debugfs repository.
How to parse multiple files:
/tools/testing/kunit/kunit.py parse results.log results2.log
How to parse all files in directory:
/tools/testing/kunit/kunit.py parse directory_path/*
How to parse KUnit debugfs repository:
/tools/testing/kunit/kunit.py parse debugfs
For each file, the parser outputs the file name, results, and test
summary. At the end of all parsing, the parser outputs a total summary
line.
This feature can be easily tested on the tools/testing/kunit/test_data/
directory.
Signed-off-by: Rae Moar <[email protected]>
---
Changes since v2:
- Fixed bug with input from command line. I changed this to use
input(). Daniel, let me know if this works for you.
- Add more specific warning messages
tools/testing/kunit/kunit.py | 56 +++++++++++++++++++++++++-----------
1 file changed, 40 insertions(+), 16 deletions(-)
diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
index bc74088c458a..1aa3d736d80c 100755
--- a/tools/testing/kunit/kunit.py
+++ b/tools/testing/kunit/kunit.py
@@ -511,19 +511,42 @@ def exec_handler(cli_args: argparse.Namespace) -> None:
def parse_handler(cli_args: argparse.Namespace) -> None:
- if cli_args.file is None:
- sys.stdin.reconfigure(errors='backslashreplace') # type: ignore
- kunit_output = sys.stdin # type: Iterable[str]
- else:
- with open(cli_args.file, 'r', errors='backslashreplace') as f:
- kunit_output = f.read().splitlines()
- # We know nothing about how the result was created!
- metadata = kunit_json.Metadata()
- request = KunitParseRequest(raw_output=cli_args.raw_output,
- json=cli_args.json)
- result, _ = parse_tests(request, metadata, kunit_output)
- if result.status != KunitStatus.SUCCESS:
- sys.exit(1)
+ parsed_files = cli_args.files # type: List[str]
+ total_test = kunit_parser.Test()
+ total_test.status = kunit_parser.TestStatus.SUCCESS
+ if not parsed_files:
+ parsed_files.append(input("File path: "))
+
+ if parsed_files[0] == "debugfs" and len(parsed_files) == 1:
+ parsed_files.pop()
+ for (root, _, files) in os.walk("/sys/kernel/debug/kunit"):
+ parsed_files.extend(os.path.join(root, f) for f in files if f == "results")
+
+ if not parsed_files:
+ print("No files found.")
+
+ for file in parsed_files:
+ if os.path.isfile(file):
+ print(file)
+ with open(file, 'r', errors='backslashreplace') as f:
+ kunit_output = f.read().splitlines()
+ # We know nothing about how the result was created!
+ metadata = kunit_json.Metadata()
+ request = KunitParseRequest(raw_output=cli_args.raw_output,
+ json=cli_args.json)
+ _, test = parse_tests(request, metadata, kunit_output)
+ total_test.subtests.append(test)
+ elif os.path.isdir(file):
+ print("Ignoring directory ", file)
+ else:
+ print("Could not find ", file)
+
+ if len(parsed_files) > 1: # if more than one file was parsed output total summary
+ print('All files parsed.')
+ if not request.raw_output:
+ stdout.print_with_timestamp(kunit_parser.DIVIDER)
+ kunit_parser.bubble_up_test_results(total_test)
+ kunit_parser.print_summary_line(total_test)
subcommand_handlers_map = {
@@ -569,9 +592,10 @@ def main(argv: Sequence[str]) -> None:
help='Parses KUnit results from a file, '
'and parses formatted results.')
add_parse_opts(parse_parser)
- parse_parser.add_argument('file',
- help='Specifies the file to read results from.',
- type=str, nargs='?', metavar='input_file')
+ parse_parser.add_argument('files',
+ help='List of file paths to read results from or keyword'
+ '"debugfs" to read all results from the debugfs directory.',
+ type=str, nargs='*', metavar='input_files')
cli_args = parser.parse_args(massage_argv(argv))
base-commit: 806cb2270237ce2ec672a407d66cee17a07d3aa2
--
2.44.0.278.ge034bb2e1d-goog
On Fri, Mar 15, 2024 at 7:16 PM Daniel Latypov <[email protected]> wrote:
>
> On Thu, Mar 7, 2024 at 2:29 PM Rae Moar <[email protected]> wrote:
> >
> > Add ability to parse multiple files. Additionally add the
> > ability to parse all results in the KUnit debugfs repository.
> >
> > How to parse multiple files:
> >
> > ./tools/testing/kunit/kunit.py parse results.log results2.log
> >
> > How to parse all files in directory:
> >
> > ./tools/testing/kunit/kunit.py parse directory_path/*
> >
> > How to parse KUnit debugfs repository:
> >
> > ./tools/testing/kunit/kunit.py parse debugfs
> >
> > For each file, the parser outputs the file name, results, and test
> > summary. At the end of all parsing, the parser outputs a total summary
> > line.
> >
> > This feature can be easily tested on the tools/testing/kunit/test_data/
> > directory.
> >
> > Signed-off-by: Rae Moar <[email protected]>
> > ---
> > Changes since v2:
> > - Fixed bug with input from command line. I changed this to use
> > input(). Daniel, let me know if this works for you.
>
> Oops, sorry for the delay.
Hi!
No worries at all. Thanks for the review!
>
> Hmm, it seems to be treating the stdin lines like file names
>
> $ ./tools/testing/kunit/kunit.py parse <
> ./tools/testing/kunit/test_data/test_config_printk_time.log
> File path: Could not find [ 0.060000] printk: console [mc-1] enabled
>
> Oh, I see, we're prompting the user via
> input("File path: ")
> ?
>
> I'm not necessarily against such a change, but I would personally
> prefer the old behavior of being able to read ktap from stdin
> directly.
> As a user, I'd also prefer to only type out filenames as arguments
> where I can get autocomplete, so `input()` here wouldn't help me
> personally.
>
> Applying a hackish patch like this [1] on top gets the behavior I'd
> personally expect:
> $ ./tools/testing/kunit/kunit.py parse <
> ./tools/testing/kunit/test_data/test_config_printk_time.log
> /dev/stdin
> ...
> [16:01:50] Testing complete. Ran 10 tests: passed: 10
>
> I'd mentioned in the previous version that we could have parsed files
> contain a `Union[str, TextIO]` and then read from the `sys.stdin` file
> object directly.
> But having it blindly open `/dev/stdin` seems to work just the same,
> if we want to keep our list simpler and just hold strings.
>
I definitely see why the change to stdin would be better. My original
change to input() was to keep it simple. But I really like the change
listed below. I will go ahead and implement that.
> [1] this just also re-orders the `os.path.isdir()` check as mentioned
> below, which simplifies things
> diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
> index 1aa3d736d80c..311d107bd684 100755
> --- a/tools/testing/kunit/kunit.py
> +++ b/tools/testing/kunit/kunit.py
> @@ -515,18 +515,18 @@ def parse_handler(cli_args: argparse.Namespace) -> None:
> total_test = kunit_parser.Test()
> total_test.status = kunit_parser.TestStatus.SUCCESS
> if not parsed_files:
> - parsed_files.append(input("File path: "))
> -
> - if parsed_files[0] == "debugfs" and len(parsed_files) == 1:
> + parsed_files.append('/dev/stdin')
> + elif len(parsed_files) == 1 and parsed_files[0] == "debugfs":
> parsed_files.pop()
> for (root, _, files) in os.walk("/sys/kernel/debug/kunit"):
> parsed_files.extend(os.path.join(root, f) for
> f in files if f == "results")
> -
> - if not parsed_files:
> - print("No files found.")
> + if not parsed_files:
> + print("No files found.")
>
> for file in parsed_files:
> - if os.path.isfile(file):
> + if os.path.isdir(file):
> + print("Ignoring directory ", file)
> + elif os.path.exists(file):
> print(file)
> with open(file, 'r', errors='backslashreplace') as f:
> kunit_output = f.read().splitlines()
> @@ -536,8 +536,6 @@ def parse_handler(cli_args: argparse.Namespace) -> None:
> json=cli_args.json)
> _, test = parse_tests(request, metadata, kunit_output)
> total_test.subtests.append(test)
> - elif os.path.isdir(file):
> - print("Ignoring directory ", file)
> else:
> print("Could not find ", file)
>
>
> > - Add more specific warning messages
> >
> > tools/testing/kunit/kunit.py | 56 +++++++++++++++++++++++++-----------
> > 1 file changed, 40 insertions(+), 16 deletions(-)
> >
> > diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
> > index bc74088c458a..1aa3d736d80c 100755
> > --- a/tools/testing/kunit/kunit.py
> > +++ b/tools/testing/kunit/kunit.py
> > @@ -511,19 +511,42 @@ def exec_handler(cli_args: argparse.Namespace) -> None:
> >
> >
> > def parse_handler(cli_args: argparse.Namespace) -> None:
> > - if cli_args.file is None:
> > - sys.stdin.reconfigure(errors='backslashreplace') # type: ignore
> > - kunit_output = sys.stdin # type: Iterable[str]
> > - else:
> > - with open(cli_args.file, 'r', errors='backslashreplace') as f:
> > - kunit_output = f.read().splitlines()
> > - # We know nothing about how the result was created!
> > - metadata = kunit_json.Metadata()
> > - request = KunitParseRequest(raw_output=cli_args.raw_output,
> > - json=cli_args.json)
> > - result, _ = parse_tests(request, metadata, kunit_output)
> > - if result.status != KunitStatus.SUCCESS:
> > - sys.exit(1)
> > + parsed_files = cli_args.files # type: List[str]
> > + total_test = kunit_parser.Test()
> > + total_test.status = kunit_parser.TestStatus.SUCCESS
> > + if not parsed_files:
> > + parsed_files.append(input("File path: "))
> > +
> > + if parsed_files[0] == "debugfs" and len(parsed_files) == 1:
> > + parsed_files.pop()
> > + for (root, _, files) in os.walk("/sys/kernel/debug/kunit"):
> > + parsed_files.extend(os.path.join(root, f) for f in files if f == "results")
> > +
> > + if not parsed_files:
> > + print("No files found.")
> > +
> > + for file in parsed_files:
> > + if os.path.isfile(file):
>
> Note: perhaps we should reorder this to
>
> if os.path.isdir(file):
> ...
> elif os.path.exists(file):
> ...
>
> That way this code will then start handling non-regular, yet readable
> files, like links, etc.
> That would also help out if we started passing in the magic
> "/dev/stdin" (since it's a symlink)
Oh I see. Yes I will try to implement this! Thanks!
>
> > + print(file)
> > + with open(file, 'r', errors='backslashreplace') as f:
> > + kunit_output = f.read().splitlines()
> > + # We know nothing about how the result was created!
> > + metadata = kunit_json.Metadata()
> > + request = KunitParseRequest(raw_output=cli_args.raw_output,
> > + json=cli_argsjson)
> > + _, test = parse_tests(request, metadata, kunit_output)
> > + total_test.subtests.append(test)
> > + elif os.path.isdir(file):
> > + print("Ignoring directory ", file)
>
> minor nit: `print()` will automatically put a space between arguments, e.g.
> > Ignoring directory .
> is what it'll print if I run `kunit.py parse .`
>
> It might be better to use a f-string so put quotes around it, like so
> print(f'Ignoring directory "{file}"')}
> and below,
> print(f'Could not find "{file}"')
>
Yep! Happy to change this.
> > + else:
> > + print("Could not find ", file)
> > +
> > + if len(parsed_files) > 1: # if more than one file was parsed output total summary
> > + print('All files parsed.')
> > + if not request.raw_output:
> > + stdout.print_with_timestamp(kunit_parser.DIVIDER)
> > + kunit_parser.bubble_up_test_results(total_test)
> > + kunit_parser.print_summary_line(total_test)
> >
> >
> > subcommand_handlers_map = {
> > @@ -569,9 +592,10 @@ def main(argv: Sequence[str]) -> None:
> > help='Parses KUnit results from a file, '
> > 'and parses formatted results.')
> > add_parse_opts(parse_parser)
> > - parse_parser.add_argument('file',
> > - help='Specifies the file to read results from.',
> > - type=str, nargs='?', metavar='input_file')
> > + parse_parser.add_argument('files',
> > + help='List of file paths to read results from or keyword'
> > + '"debugfs" to read all results from the debugfs directory.',
>
> minor spacing note: there are two ' 's here in the series of tabs, i.e.
> ^I^I^I^I ^I^I'"debugfs" to read all results from the debugfs directory',$
> (using vim's :list formatting)
>
> This was copy-pasted from the lines above and below which look like
> ^I^I^I^I help='List of file paths to read results from or keyword'$
> i.e. they use the 2 spaces to align after the tabs.
>
> We can just drop those 2 spaces since they won't visually affect the
> outcome with a tabwidth of 8 spaces.
Thanks for pointing this out! I will change the spacing here. I am
thinking of just changing it to match the other lines. So something
like this:
^I^I^I^I '"debugfs" to read all results from the debugfs directory.',$
>
> Sorry again for the delayed reply,
> Daniel
On Thu, Mar 7, 2024 at 2:29 PM Rae Moar <[email protected]> wrote:
>
> Add ability to parse multiple files. Additionally add the
> ability to parse all results in the KUnit debugfs repository.
>
> How to parse multiple files:
>
> ./tools/testing/kunit/kunit.py parse results.log results2.log
>
> How to parse all files in directory:
>
> ./tools/testing/kunit/kunit.py parse directory_path/*
>
> How to parse KUnit debugfs repository:
>
> ./tools/testing/kunit/kunit.py parse debugfs
>
> For each file, the parser outputs the file name, results, and test
> summary. At the end of all parsing, the parser outputs a total summary
> line.
>
> This feature can be easily tested on the tools/testing/kunit/test_data/
> directory.
>
> Signed-off-by: Rae Moar <[email protected]>
> ---
> Changes since v2:
> - Fixed bug with input from command line. I changed this to use
> input(). Daniel, let me know if this works for you.
Oops, sorry for the delay.
Hmm, it seems to be treating the stdin lines like file names
$ ./tools/testing/kunit/kunit.py parse <
/tools/testing/kunit/test_data/test_config_printk_time.log
File path: Could not find [ 0.060000] printk: console [mc-1] enabled
Oh, I see, we're prompting the user via
input("File path: ")
?
I'm not necessarily against such a change, but I would personally
prefer the old behavior of being able to read ktap from stdin
directly.
As a user, I'd also prefer to only type out filenames as arguments
where I can get autocomplete, so `input()` here wouldn't help me
personally.
Applying a hackish patch like this [1] on top gets the behavior I'd
personally expect:
$ ./tools/testing/kunit/kunit.py parse <
/tools/testing/kunit/test_data/test_config_printk_time.log
/dev/stdin
..
[16:01:50] Testing complete. Ran 10 tests: passed: 10
I'd mentioned in the previous version that we could have parsed files
contain a `Union[str, TextIO]` and then read from the `sys.stdin` file
object directly.
But having it blindly open `/dev/stdin` seems to work just the same,
if we want to keep our list simpler and just hold strings.
[1] this just also re-orders the `os.path.isdir()` check as mentioned
below, which simplifies things
diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
index 1aa3d736d80c..311d107bd684 100755
--- a/tools/testing/kunit/kunit.py
+++ b/tools/testing/kunit/kunit.py
@@ -515,18 +515,18 @@ def parse_handler(cli_args: argparse.Namespace) -> None:
total_test = kunit_parser.Test()
total_test.status = kunit_parser.TestStatus.SUCCESS
if not parsed_files:
- parsed_files.append(input("File path: "))
-
- if parsed_files[0] == "debugfs" and len(parsed_files) == 1:
+ parsed_files.append('/dev/stdin')
+ elif len(parsed_files) == 1 and parsed_files[0] == "debugfs":
parsed_files.pop()
for (root, _, files) in os.walk("/sys/kernel/debug/kunit"):
parsed_files.extend(os.path.join(root, f) for
f in files if f == "results")
-
- if not parsed_files:
- print("No files found.")
+ if not parsed_files:
+ print("No files found.")
for file in parsed_files:
- if os.path.isfile(file):
+ if os.path.isdir(file):
+ print("Ignoring directory ", file)
+ elif os.path.exists(file):
print(file)
with open(file, 'r', errors='backslashreplace') as f:
kunit_output = f.read().splitlines()
@@ -536,8 +536,6 @@ def parse_handler(cli_args: argparse.Namespace) -> None:
json=cli_args.json)
_, test = parse_tests(request, metadata, kunit_output)
total_test.subtests.append(test)
- elif os.path.isdir(file):
- print("Ignoring directory ", file)
else:
print("Could not find ", file)
> - Add more specific warning messages
>
> tools/testing/kunit/kunit.py | 56 +++++++++++++++++++++++++-----------
> 1 file changed, 40 insertions(+), 16 deletions(-)
>
> diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
> index bc74088c458a..1aa3d736d80c 100755
> --- a/tools/testing/kunit/kunit.py
> +++ b/tools/testing/kunit/kunit.py
> @@ -511,19 +511,42 @@ def exec_handler(cli_args: argparse.Namespace) -> None:
>
>
> def parse_handler(cli_args: argparse.Namespace) -> None:
> - if cli_args.file is None:
> - sys.stdin.reconfigure(errors='backslashreplace') # type: ignore
> - kunit_output = sys.stdin # type: Iterable[str]
> - else:
> - with open(cli_args.file, 'r', errors='backslashreplace') as f:
> - kunit_output = f.read().splitlines()
> - # We know nothing about how the result was created!
> - metadata = kunit_json.Metadata()
> - request = KunitParseRequest(raw_output=cli_args.raw_output,
> - json=cli_args.json)
> - result, _ = parse_tests(request, metadata, kunit_output)
> - if result.status != KunitStatus.SUCCESS:
> - sys.exit(1)
> + parsed_files = cli_args.files # type: List[str]
> + total_test = kunit_parser.Test()
> + total_test.status = kunit_parser.TestStatus.SUCCESS
> + if not parsed_files:
> + parsed_files.append(input("File path: "))
> +
> + if parsed_files[0] == "debugfs" and len(parsed_files) == 1:
> + parsed_files.pop()
> + for (root, _, files) in os.walk("/sys/kernel/debug/kunit"):
> + parsed_files.extend(os.path.join(root, f) for f in files if f == "results")
> +
> + if not parsed_files:
> + print("No files found.")
> +
> + for file in parsed_files:
> + if os.path.isfile(file):
Note: perhaps we should reorder this to
if os.path.isdir(file):
...
elif os.path.exists(file):
...
That way this code will then start handling non-regular, yet readable
files, like links, etc.
That would also help out if we started passing in the magic
"/dev/stdin" (since it's a symlink)
> + print(file)
> + with open(file, 'r', errors='backslashreplace') as f:
> + kunit_output = f.read().splitlines()
> + # We know nothing about how the result was created!
> + metadata = kunit_json.Metadata()
> + request = KunitParseRequest(raw_output=cli_args.raw_output,
> + json=cli_args.json)
> + _, test = parse_tests(request, metadata, kunit_output)
> + total_test.subtests.append(test)
> + elif os.path.isdir(file):
> + print("Ignoring directory ", file)
minor nit: `print()` will automatically put a space between arguments, e.g.
> Ignoring directory .
is what it'll print if I run `kunit.py parse .`
It might be better to use a f-string so put quotes around it, like so
print(f'Ignoring directory "{file}"')}
and below,
print(f'Could not find "{file}"')
> + else:
> + print("Could not find ", file)
> +
> + if len(parsed_files) > 1: # if more than one file was parsed output total summary
> + print('All files parsed.')
> + if not request.raw_output:
> + stdout.print_with_timestamp(kunit_parser.DIVIDER)
> + kunit_parser.bubble_up_test_results(total_test)
> + kunit_parser.print_summary_line(total_test)
>
>
> subcommand_handlers_map = {
> @@ -569,9 +592,10 @@ def main(argv: Sequence[str]) -> None:
> help='Parses KUnit results from a file, '
> 'and parses formatted results')
> add_parse_opts(parse_parser)
> - parse_parser.add_argument('file',
> - help='Specifies the file to read results from.',
> - type=str, nargs='?', metavar='input_file')
> + parse_parser.add_argument('files',
> + help='List of file paths to read results from or keyword'
> + '"debugfs" to read all results from the debugfs directory.',
minor spacing note: there are two ' 's here in the series of tabs, i.e.
^I^I^I^I ^I^I'"debugfs" to read all results from the debugfs directory.',$
(using vim's :list formatting)
This was copy-pasted from the lines above and below which look like
^I^I^I^I help='List of file paths to read results from or keyword'$
i.e. they use the 2 spaces to align after the tabs.
We can just drop those 2 spaces since they won't visually affect the
outcome with a tabwidth of 8 spaces.
Sorry again for the delayed reply,
Daniel