* [PATCH v1 1/2] kunit: tool: fix running kunit_tool from outside kernel tree @ 2020-08-08 1:16 Brendan Higgins 2020-08-08 1:16 ` [PATCH v1 2/2] kunit: tool: allow generating test results in JSON Brendan Higgins 2020-08-08 5:45 ` [PATCH v1 1/2] kunit: tool: fix running kunit_tool from outside kernel tree David Gow 0 siblings, 2 replies; 7+ messages in thread From: Brendan Higgins @ 2020-08-08 1:16 UTC (permalink / raw) To: shuah, davidgow; +Cc: linux-kselftest, kunit-dev, linux-kernel, Brendan Higgins Currently kunit_tool does not work correctly when executed from a path outside of the kernel tree, so make sure that the current working directory is correct and the kunit_dir is properly initialized before running. Signed-off-by: Brendan Higgins <brendanhiggins@google.com> --- tools/testing/kunit/kunit.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py index 425ef40067e7..96344a11ff1f 100755 --- a/tools/testing/kunit/kunit.py +++ b/tools/testing/kunit/kunit.py @@ -237,9 +237,14 @@ def main(argv, linux=None): cli_args = parser.parse_args(argv) + if get_kernel_root_path(): + print('cd ' + get_kernel_root_path()) + os.chdir(get_kernel_root_path()) + if cli_args.subcommand == 'run': if not os.path.exists(cli_args.build_dir): os.mkdir(cli_args.build_dir) + create_default_kunitconfig() if not linux: linux = kunit_kernel.LinuxSourceTree() @@ -257,6 +262,7 @@ def main(argv, linux=None): if cli_args.build_dir: if not os.path.exists(cli_args.build_dir): os.mkdir(cli_args.build_dir) + create_default_kunitconfig() if not linux: linux = kunit_kernel.LinuxSourceTree() @@ -273,6 +279,7 @@ def main(argv, linux=None): if cli_args.build_dir: if not os.path.exists(cli_args.build_dir): os.mkdir(cli_args.build_dir) + create_default_kunitconfig() if not linux: linux = kunit_kernel.LinuxSourceTree() @@ -291,6 +298,7 @@ def main(argv, linux=None): if cli_args.build_dir: if not os.path.exists(cli_args.build_dir): os.mkdir(cli_args.build_dir) + create_default_kunitconfig() if not linux: linux = kunit_kernel.LinuxSourceTree() base-commit: 30185b69a2d533c4ba6ca926b8390ce7de495e29 -- 2.28.0.236.gb10cc79966-goog ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v1 2/2] kunit: tool: allow generating test results in JSON 2020-08-08 1:16 [PATCH v1 1/2] kunit: tool: fix running kunit_tool from outside kernel tree Brendan Higgins @ 2020-08-08 1:16 ` Brendan Higgins 2020-08-11 19:56 ` Bird, Tim 2020-08-08 5:45 ` [PATCH v1 1/2] kunit: tool: fix running kunit_tool from outside kernel tree David Gow 1 sibling, 1 reply; 7+ messages in thread From: Brendan Higgins @ 2020-08-08 1:16 UTC (permalink / raw) To: shuah, davidgow Cc: linux-kselftest, kunit-dev, linux-kernel, Heidi Fahim, Brendan Higgins From: Heidi Fahim <heidifahim@google.com> Add a --json flag, which when specified when kunit_tool is run, generates JSON formatted test results conforming to the KernelCI API test_group spec[1]. The user can the new flag to specify a filename as the value to json in order to store the JSON results under linux/. Link[1]: https://api.kernelci.org/schema-test-group.html#post Signed-off-by: Heidi Fahim <heidifahim@google.com> Signed-off-by: Brendan Higgins <brendanhiggins@google.com> --- tools/testing/kunit/kunit.py | 35 +++++++++++--- tools/testing/kunit/kunit_json.py | 63 ++++++++++++++++++++++++++ tools/testing/kunit/kunit_tool_test.py | 33 ++++++++++++++ 3 files changed, 125 insertions(+), 6 deletions(-) create mode 100644 tools/testing/kunit/kunit_json.py diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py index 96344a11ff1f..485b7c63b967 100755 --- a/tools/testing/kunit/kunit.py +++ b/tools/testing/kunit/kunit.py @@ -17,6 +17,7 @@ from collections import namedtuple from enum import Enum, auto import kunit_config +import kunit_json import kunit_kernel import kunit_parser @@ -30,9 +31,9 @@ KunitBuildRequest = namedtuple('KunitBuildRequest', KunitExecRequest = namedtuple('KunitExecRequest', ['timeout', 'build_dir', 'alltests']) KunitParseRequest = namedtuple('KunitParseRequest', - ['raw_output', 'input_data']) + ['raw_output', 'input_data', 'build_dir', 'json']) KunitRequest = namedtuple('KunitRequest', ['raw_output','timeout', 'jobs', - 'build_dir', 'alltests', + 'build_dir', 'alltests', 'json', 'make_options']) KernelDirectoryPath = sys.argv[0].split('tools/testing/kunit/')[0] @@ -113,12 +114,22 @@ def parse_tests(request: KunitParseRequest) -> KunitResult: test_result = kunit_parser.TestResult(kunit_parser.TestStatus.SUCCESS, [], 'Tests not Parsed.') + if request.raw_output: kunit_parser.raw_output(request.input_data) else: test_result = kunit_parser.parse_run_tests(request.input_data) parse_end = time.time() + if request.json: + json_obj = kunit_json.get_json_result( + test_result=test_result, + def_config='kunit_defconfig', + build_dir=request.build_dir, + json_path=request.json) + if request.json == 'stdout': + print(json_obj) + if test_result.status != kunit_parser.TestStatus.SUCCESS: return KunitResult(KunitStatus.TEST_FAILURE, test_result, parse_end - parse_start) @@ -151,7 +162,9 @@ def run_tests(linux: kunit_kernel.LinuxSourceTree, return exec_result parse_request = KunitParseRequest(request.raw_output, - exec_result.result) + exec_result.result, + request.build_dir, + request.json) parse_result = parse_tests(parse_request) run_end = time.time() @@ -195,7 +208,12 @@ def add_exec_opts(parser): def add_parse_opts(parser): parser.add_argument('--raw_output', help='don\'t format output from kernel', action='store_true') - + parser.add_argument('--json', + nargs='?', + help='Stores test results in a JSON, and either ' + 'prints to stdout or saves to file if a ' + 'filename is specified', + type=str, const='stdout', default=None) def main(argv, linux=None): parser = argparse.ArgumentParser( @@ -254,6 +272,7 @@ def main(argv, linux=None): cli_args.jobs, cli_args.build_dir, cli_args.alltests, + cli_args.json, cli_args.make_options) result = run_tests(linux, request) if result.status != KunitStatus.SUCCESS: @@ -308,7 +327,9 @@ def main(argv, linux=None): cli_args.alltests) exec_result = exec_tests(linux, exec_request) parse_request = KunitParseRequest(cli_args.raw_output, - exec_result.result) + exec_result.result, + cli_args.build_dir, + cli_args.json) result = parse_tests(parse_request) kunit_parser.print_with_timestamp(( 'Elapsed time: %.3fs\n') % ( @@ -322,7 +343,9 @@ def main(argv, linux=None): with open(cli_args.file, 'r') as f: kunit_output = f.read().splitlines() request = KunitParseRequest(cli_args.raw_output, - kunit_output) + kunit_output, + cli_args.build_dir, + cli_args.json) result = parse_tests(request) if result.status != KunitStatus.SUCCESS: sys.exit(1) diff --git a/tools/testing/kunit/kunit_json.py b/tools/testing/kunit/kunit_json.py new file mode 100644 index 000000000000..624b31b2dbd6 --- /dev/null +++ b/tools/testing/kunit/kunit_json.py @@ -0,0 +1,63 @@ +# SPDX-License-Identifier: GPL-2.0 +# +# Generates JSON from KUnit results according to +# KernelCI spec: https://github.com/kernelci/kernelci-doc/wiki/Test-API +# +# Copyright (C) 2020, Google LLC. +# Author: Heidi Fahim <heidifahim@google.com> + +import json +import os + +import kunit_parser + +from kunit_parser import TestStatus + +def get_json_result(test_result, def_config, build_dir, json_path): + sub_groups = [] + + # Each test suite is mapped to a KernelCI sub_group + for test_suite in test_result.suites: + sub_group = { + "name": test_suite.name, + "arch": "UM", + "defconfig": def_config, + "build_environment": build_dir, + "test_cases": [], + "lab_name": None, + "kernel": None, + "job": None, + "git_branch": "kselftest", + } + test_cases = [] + # TODO: Add attachments attribute in test_case with detailed + # failure message, see https://api.kernelci.org/schema-test-case.html#get + for case in test_suite.cases: + test_case = {"name": case.name, "status": "FAIL"} + if case.status == TestStatus.SUCCESS: + test_case["status"] = "PASS" + elif case.status == TestStatus.TEST_CRASHED: + test_case["status"] = "ERROR" + test_cases.append(test_case) + sub_group["test_cases"] = test_cases + sub_groups.append(sub_group) + test_group = { + "name": "KUnit Test Group", + "arch": "UM", + "defconfig": def_config, + "build_environment": build_dir, + "sub_groups": sub_groups, + "lab_name": None, + "kernel": None, + "job": None, + "git_branch": "kselftest", + } + json_obj = json.dumps(test_group, indent=4) + if json_path != 'stdout': + with open(json_path, 'w') as result_path: + result_path.write(json_obj) + root = __file__.split('tools/testing/kunit/')[0] + kunit_parser.print_with_timestamp( + "Test results stored in %s" % + os.path.join(root, result_path.name)) + return json_obj diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py index 287c74d821c3..99c3c5671ea4 100755 --- a/tools/testing/kunit/kunit_tool_test.py +++ b/tools/testing/kunit/kunit_tool_test.py @@ -11,11 +11,13 @@ from unittest import mock import tempfile, shutil # Handling test_tmpdir +import json import os import kunit_config import kunit_parser import kunit_kernel +import kunit_json import kunit test_tmpdir = '' @@ -230,6 +232,37 @@ class KUnitParserTest(unittest.TestCase): result = kunit_parser.parse_run_tests(file.readlines()) self.assertEqual('kunit-resource-test', result.suites[0].name) +class KUnitJsonTest(unittest.TestCase): + + def _json_for(self, log_file): + with(open(get_absolute_path(log_file))) as file: + test_result = kunit_parser.parse_run_tests(file) + json_obj = kunit_json.get_json_result( + test_result=test_result, + def_config='kunit_defconfig', + build_dir=None, + json_path='stdout') + return json.loads(json_obj) + + def test_failed_test_json(self): + result = self._json_for( + 'test_data/test_is_test_passed-failure.log') + self.assertEqual( + {'name': 'example_simple_test', 'status': 'FAIL'}, + result["sub_groups"][1]["test_cases"][0]) + + def test_crashed_test_json(self): + result = self._json_for( + 'test_data/test_is_test_passed-crash.log') + self.assertEqual( + {'name': 'example_simple_test', 'status': 'ERROR'}, + result["sub_groups"][1]["test_cases"][0]) + + def test_no_tests_json(self): + result = self._json_for( + 'test_data/test_is_test_passed-no_tests_run.log') + self.assertEqual(0, len(result['sub_groups'])) + class StrContains(str): def __eq__(self, other): return self in other -- 2.28.0.236.gb10cc79966-goog ^ permalink raw reply related [flat|nested] 7+ messages in thread
* RE: [PATCH v1 2/2] kunit: tool: allow generating test results in JSON 2020-08-08 1:16 ` [PATCH v1 2/2] kunit: tool: allow generating test results in JSON Brendan Higgins @ 2020-08-11 19:56 ` Bird, Tim 2020-08-11 20:00 ` Brendan Higgins 0 siblings, 1 reply; 7+ messages in thread From: Bird, Tim @ 2020-08-11 19:56 UTC (permalink / raw) To: Brendan Higgins, shuah, davidgow Cc: linux-kselftest, kunit-dev, linux-kernel, Heidi Fahim > -----Original Message----- > From: Brendan Higgins > Sent: Friday, August 7, 2020 7:17 PM > > From: Heidi Fahim <heidifahim@google.com> > > Add a --json flag, which when specified when kunit_tool is run, > generates JSON formatted test results conforming to the KernelCI API > test_group spec[1]. The user can the new flag to specify a filename as Seems to be missing a word. "The user can ? the new flag" > the value to json in order to store the JSON results under linux/. > > Link[1]: https://api.kernelci.org/schema-test-group.html#post > Signed-off-by: Heidi Fahim <heidifahim@google.com> > Signed-off-by: Brendan Higgins <brendanhiggins@google.com> > --- > tools/testing/kunit/kunit.py | 35 +++++++++++--- > tools/testing/kunit/kunit_json.py | 63 ++++++++++++++++++++++++++ > tools/testing/kunit/kunit_tool_test.py | 33 ++++++++++++++ > 3 files changed, 125 insertions(+), 6 deletions(-) > create mode 100644 tools/testing/kunit/kunit_json.py > > diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py > index 96344a11ff1f..485b7c63b967 100755 > --- a/tools/testing/kunit/kunit.py > +++ b/tools/testing/kunit/kunit.py > @@ -17,6 +17,7 @@ from collections import namedtuple > from enum import Enum, auto > > import kunit_config > +import kunit_json > import kunit_kernel > import kunit_parser > > @@ -30,9 +31,9 @@ KunitBuildRequest = namedtuple('KunitBuildRequest', > KunitExecRequest = namedtuple('KunitExecRequest', > ['timeout', 'build_dir', 'alltests']) > KunitParseRequest = namedtuple('KunitParseRequest', > - ['raw_output', 'input_data']) > + ['raw_output', 'input_data', 'build_dir', 'json']) > KunitRequest = namedtuple('KunitRequest', ['raw_output','timeout', 'jobs', > - 'build_dir', 'alltests', > + 'build_dir', 'alltests', 'json', > 'make_options']) > > KernelDirectoryPath = sys.argv[0].split('tools/testing/kunit/')[0] > @@ -113,12 +114,22 @@ def parse_tests(request: KunitParseRequest) -> KunitResult: > test_result = kunit_parser.TestResult(kunit_parser.TestStatus.SUCCESS, > [], > 'Tests not Parsed.') > + > if request.raw_output: > kunit_parser.raw_output(request.input_data) > else: > test_result = kunit_parser.parse_run_tests(request.input_data) > parse_end = time.time() > > + if request.json: > + json_obj = kunit_json.get_json_result( > + test_result=test_result, > + def_config='kunit_defconfig', > + build_dir=request.build_dir, > + json_path=request.json) > + if request.json == 'stdout': > + print(json_obj) > + > if test_result.status != kunit_parser.TestStatus.SUCCESS: > return KunitResult(KunitStatus.TEST_FAILURE, test_result, > parse_end - parse_start) > @@ -151,7 +162,9 @@ def run_tests(linux: kunit_kernel.LinuxSourceTree, > return exec_result > > parse_request = KunitParseRequest(request.raw_output, > - exec_result.result) > + exec_result.result, > + request.build_dir, > + request.json) > parse_result = parse_tests(parse_request) > > run_end = time.time() > @@ -195,7 +208,12 @@ def add_exec_opts(parser): > def add_parse_opts(parser): > parser.add_argument('--raw_output', help='don\'t format output from kernel', > action='store_true') > - > + parser.add_argument('--json', > + nargs='?', > + help='Stores test results in a JSON, and either ' > + 'prints to stdout or saves to file if a ' > + 'filename is specified', > + type=str, const='stdout', default=None) > > def main(argv, linux=None): > parser = argparse.ArgumentParser( > @@ -254,6 +272,7 @@ def main(argv, linux=None): > cli_args.jobs, > cli_args.build_dir, > cli_args.alltests, > + cli_args.json, > cli_args.make_options) > result = run_tests(linux, request) > if result.status != KunitStatus.SUCCESS: > @@ -308,7 +327,9 @@ def main(argv, linux=None): > cli_args.alltests) > exec_result = exec_tests(linux, exec_request) > parse_request = KunitParseRequest(cli_args.raw_output, > - exec_result.result) > + exec_result.result, > + cli_args.build_dir, > + cli_args.json) > result = parse_tests(parse_request) > kunit_parser.print_with_timestamp(( > 'Elapsed time: %.3fs\n') % ( > @@ -322,7 +343,9 @@ def main(argv, linux=None): > with open(cli_args.file, 'r') as f: > kunit_output = f.read().splitlines() > request = KunitParseRequest(cli_args.raw_output, > - kunit_output) > + kunit_output, > + cli_args.build_dir, > + cli_args.json) > result = parse_tests(request) > if result.status != KunitStatus.SUCCESS: > sys.exit(1) > diff --git a/tools/testing/kunit/kunit_json.py b/tools/testing/kunit/kunit_json.py > new file mode 100644 > index 000000000000..624b31b2dbd6 > --- /dev/null > +++ b/tools/testing/kunit/kunit_json.py > @@ -0,0 +1,63 @@ > +# SPDX-License-Identifier: GPL-2.0 > +# > +# Generates JSON from KUnit results according to > +# KernelCI spec: https://github.com/kernelci/kernelci-doc/wiki/Test-API > +# > +# Copyright (C) 2020, Google LLC. > +# Author: Heidi Fahim <heidifahim@google.com> > + > +import json > +import os > + > +import kunit_parser > + > +from kunit_parser import TestStatus > + > +def get_json_result(test_result, def_config, build_dir, json_path): > + sub_groups = [] > + > + # Each test suite is mapped to a KernelCI sub_group > + for test_suite in test_result.suites: > + sub_group = { > + "name": test_suite.name, > + "arch": "UM", > + "defconfig": def_config, > + "build_environment": build_dir, > + "test_cases": [], > + "lab_name": None, > + "kernel": None, > + "job": None, > + "git_branch": "kselftest", > + } > + test_cases = [] > + # TODO: Add attachments attribute in test_case with detailed > + # failure message, see https://api.kernelci.org/schema-test-case.html#get > + for case in test_suite.cases: > + test_case = {"name": case.name, "status": "FAIL"} > + if case.status == TestStatus.SUCCESS: > + test_case["status"] = "PASS" > + elif case.status == TestStatus.TEST_CRASHED: > + test_case["status"] = "ERROR" > + test_cases.append(test_case) > + sub_group["test_cases"] = test_cases > + sub_groups.append(sub_group) > + test_group = { > + "name": "KUnit Test Group", > + "arch": "UM", > + "defconfig": def_config, > + "build_environment": build_dir, > + "sub_groups": sub_groups, > + "lab_name": None, > + "kernel": None, > + "job": None, > + "git_branch": "kselftest", > + } > + json_obj = json.dumps(test_group, indent=4) > + if json_path != 'stdout': > + with open(json_path, 'w') as result_path: > + result_path.write(json_obj) > + root = __file__.split('tools/testing/kunit/')[0] > + kunit_parser.print_with_timestamp( > + "Test results stored in %s" % > + os.path.join(root, result_path.name)) > + return json_obj > diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py > index 287c74d821c3..99c3c5671ea4 100755 > --- a/tools/testing/kunit/kunit_tool_test.py > +++ b/tools/testing/kunit/kunit_tool_test.py > @@ -11,11 +11,13 @@ from unittest import mock > > import tempfile, shutil # Handling test_tmpdir > > +import json > import os > > import kunit_config > import kunit_parser > import kunit_kernel > +import kunit_json > import kunit > > test_tmpdir = '' > @@ -230,6 +232,37 @@ class KUnitParserTest(unittest.TestCase): > result = kunit_parser.parse_run_tests(file.readlines()) > self.assertEqual('kunit-resource-test', result.suites[0].name) > > +class KUnitJsonTest(unittest.TestCase): > + > + def _json_for(self, log_file): > + with(open(get_absolute_path(log_file))) as file: > + test_result = kunit_parser.parse_run_tests(file) > + json_obj = kunit_json.get_json_result( > + test_result=test_result, > + def_config='kunit_defconfig', > + build_dir=None, > + json_path='stdout') > + return json.loads(json_obj) > + > + def test_failed_test_json(self): > + result = self._json_for( > + 'test_data/test_is_test_passed-failure.log') > + self.assertEqual( > + {'name': 'example_simple_test', 'status': 'FAIL'}, > + result["sub_groups"][1]["test_cases"][0]) > + > + def test_crashed_test_json(self): > + result = self._json_for( > + 'test_data/test_is_test_passed-crash.log') > + self.assertEqual( > + {'name': 'example_simple_test', 'status': 'ERROR'}, > + result["sub_groups"][1]["test_cases"][0]) > + > + def test_no_tests_json(self): > + result = self._json_for( > + 'test_data/test_is_test_passed-no_tests_run.log') > + self.assertEqual(0, len(result['sub_groups'])) > + > class StrContains(str): > def __eq__(self, other): > return self in other > -- > 2.28.0.236.gb10cc79966-goog ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1 2/2] kunit: tool: allow generating test results in JSON 2020-08-11 19:56 ` Bird, Tim @ 2020-08-11 20:00 ` Brendan Higgins 0 siblings, 0 replies; 7+ messages in thread From: Brendan Higgins @ 2020-08-11 20:00 UTC (permalink / raw) To: Bird, Tim Cc: shuah, davidgow, linux-kselftest, kunit-dev, linux-kernel, Heidi Fahim On Tue, Aug 11, 2020 at 12:56 PM Bird, Tim <Tim.Bird@sony.com> wrote: > > > > > -----Original Message----- > > From: Brendan Higgins > > Sent: Friday, August 7, 2020 7:17 PM > > > > From: Heidi Fahim <heidifahim@google.com> > > > > Add a --json flag, which when specified when kunit_tool is run, > > generates JSON formatted test results conforming to the KernelCI API > > test_group spec[1]. The user can the new flag to specify a filename as > > Seems to be missing a word. "The user can ? the new flag" Whoops, good catch. I will fix it in the next revision. > > the value to json in order to store the JSON results under linux/. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1 1/2] kunit: tool: fix running kunit_tool from outside kernel tree 2020-08-08 1:16 [PATCH v1 1/2] kunit: tool: fix running kunit_tool from outside kernel tree Brendan Higgins 2020-08-08 1:16 ` [PATCH v1 2/2] kunit: tool: allow generating test results in JSON Brendan Higgins @ 2020-08-08 5:45 ` David Gow 2020-08-08 8:50 ` Brendan Higgins 1 sibling, 1 reply; 7+ messages in thread From: David Gow @ 2020-08-08 5:45 UTC (permalink / raw) To: Brendan Higgins Cc: Shuah Khan, open list:KERNEL SELFTEST FRAMEWORK, KUnit Development, Linux Kernel Mailing List On Sat, Aug 8, 2020 at 9:17 AM Brendan Higgins <brendanhiggins@google.com> wrote: > > Currently kunit_tool does not work correctly when executed from a path > outside of the kernel tree, so make sure that the current working > directory is correct and the kunit_dir is properly initialized before > running. > > Signed-off-by: Brendan Higgins <brendanhiggins@google.com> > --- > tools/testing/kunit/kunit.py | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py > index 425ef40067e7..96344a11ff1f 100755 > --- a/tools/testing/kunit/kunit.py > +++ b/tools/testing/kunit/kunit.py > @@ -237,9 +237,14 @@ def main(argv, linux=None): > > cli_args = parser.parse_args(argv) > > + if get_kernel_root_path(): > + print('cd ' + get_kernel_root_path()) Do we want to print this, or is it a leftover debug statement? > + os.chdir(get_kernel_root_path()) > + > if cli_args.subcommand == 'run': > if not os.path.exists(cli_args.build_dir): > os.mkdir(cli_args.build_dir) > + create_default_kunitconfig() Why are we adding this everywhere when it's already in config_tests, which should already be called in all of the places where a kunitconfig is required? Is the goal to always copy the default kunitconfig when creating a new build_dir? While I can sort-of see why we might want to do that, if the build dir doesn't exist, most of the subcommands will fail anyway (maybe we should only create the build-dir for 'config' and 'run'?) > > if not linux: > linux = kunit_kernel.LinuxSourceTree() > @@ -257,6 +262,7 @@ def main(argv, linux=None): > if cli_args.build_dir: > if not os.path.exists(cli_args.build_dir): > os.mkdir(cli_args.build_dir) > + create_default_kunitconfig() > > if not linux: > linux = kunit_kernel.LinuxSourceTree() > @@ -273,6 +279,7 @@ def main(argv, linux=None): > if cli_args.build_dir: > if not os.path.exists(cli_args.build_dir): > os.mkdir(cli_args.build_dir) > + create_default_kunitconfig() > > if not linux: > linux = kunit_kernel.LinuxSourceTree() > @@ -291,6 +298,7 @@ def main(argv, linux=None): > if cli_args.build_dir: > if not os.path.exists(cli_args.build_dir): > os.mkdir(cli_args.build_dir) > + create_default_kunitconfig() > > if not linux: > linux = kunit_kernel.LinuxSourceTree() > > base-commit: 30185b69a2d533c4ba6ca926b8390ce7de495e29 > -- > 2.28.0.236.gb10cc79966-goog > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1 1/2] kunit: tool: fix running kunit_tool from outside kernel tree 2020-08-08 5:45 ` [PATCH v1 1/2] kunit: tool: fix running kunit_tool from outside kernel tree David Gow @ 2020-08-08 8:50 ` Brendan Higgins 2020-08-11 4:29 ` David Gow 0 siblings, 1 reply; 7+ messages in thread From: Brendan Higgins @ 2020-08-08 8:50 UTC (permalink / raw) To: David Gow Cc: Shuah Khan, open list:KERNEL SELFTEST FRAMEWORK, KUnit Development, Linux Kernel Mailing List On Fri, Aug 7, 2020 at 10:45 PM David Gow <davidgow@google.com> wrote: > > On Sat, Aug 8, 2020 at 9:17 AM Brendan Higgins > <brendanhiggins@google.com> wrote: > > > > Currently kunit_tool does not work correctly when executed from a path > > outside of the kernel tree, so make sure that the current working > > directory is correct and the kunit_dir is properly initialized before > > running. > > > > Signed-off-by: Brendan Higgins <brendanhiggins@google.com> > > --- > > tools/testing/kunit/kunit.py | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py > > index 425ef40067e7..96344a11ff1f 100755 > > --- a/tools/testing/kunit/kunit.py > > +++ b/tools/testing/kunit/kunit.py > > @@ -237,9 +237,14 @@ def main(argv, linux=None): > > > > cli_args = parser.parse_args(argv) > > > > + if get_kernel_root_path(): > > + print('cd ' + get_kernel_root_path()) > Do we want to print this, or is it a leftover debug statement? Whoops, I was supposed to delete that. That's embarrassing... ^_^; > > + os.chdir(get_kernel_root_path()) > > + > > if cli_args.subcommand == 'run': > > if not os.path.exists(cli_args.build_dir): > > os.mkdir(cli_args.build_dir) > > + create_default_kunitconfig() > Why are we adding this everywhere when it's already in config_tests, > which should already be called in all of the places where a > kunitconfig is required? Ah yes, .kunitconfig needs to be created before config_tests() can be called because the LinuxSourceTree constructor needs .kunitconfig to exist. > Is the goal to always copy the default kunitconfig when creating a new > build_dir? While I can sort-of see why we might want to do that, if > the build dir doesn't exist, most of the subcommands will fail anyway > (maybe we should only create the build-dir for 'config' and 'run'?) I just did it because we were getting a failure in a constructor so we couldn't do much. Ideally we would check that the current state allows for the command that the user intended to run, but I think that's beyond the scope of this change. So I guess the real question is: Is it okay for it to crash in the constructor with a cryptic error message for now, or do we want to let it fail with a slightly less cryptic message later? > > if not linux: > > linux = kunit_kernel.LinuxSourceTree() > > @@ -257,6 +262,7 @@ def main(argv, linux=None): > > if cli_args.build_dir: > > if not os.path.exists(cli_args.build_dir): > > os.mkdir(cli_args.build_dir) > > + create_default_kunitconfig() > > > > if not linux: > > linux = kunit_kernel.LinuxSourceTree() > > @@ -273,6 +279,7 @@ def main(argv, linux=None): > > if cli_args.build_dir: > > if not os.path.exists(cli_args.build_dir): > > os.mkdir(cli_args.build_dir) > > + create_default_kunitconfig() > > > > if not linux: > > linux = kunit_kernel.LinuxSourceTree() > > @@ -291,6 +298,7 @@ def main(argv, linux=None): > > if cli_args.build_dir: > > if not os.path.exists(cli_args.build_dir): > > os.mkdir(cli_args.build_dir) > > + create_default_kunitconfig() > > > > if not linux: > > linux = kunit_kernel.LinuxSourceTree() > > > > base-commit: 30185b69a2d533c4ba6ca926b8390ce7de495e29 > > -- > > 2.28.0.236.gb10cc79966-goog > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1 1/2] kunit: tool: fix running kunit_tool from outside kernel tree 2020-08-08 8:50 ` Brendan Higgins @ 2020-08-11 4:29 ` David Gow 0 siblings, 0 replies; 7+ messages in thread From: David Gow @ 2020-08-11 4:29 UTC (permalink / raw) To: Brendan Higgins Cc: Shuah Khan, open list:KERNEL SELFTEST FRAMEWORK, KUnit Development, Linux Kernel Mailing List On Sat, Aug 8, 2020 at 4:51 PM Brendan Higgins <brendanhiggins@google.com> wrote: > > On Fri, Aug 7, 2020 at 10:45 PM David Gow <davidgow@google.com> wrote: > > > > On Sat, Aug 8, 2020 at 9:17 AM Brendan Higgins > > <brendanhiggins@google.com> wrote: > > > > > > Currently kunit_tool does not work correctly when executed from a path > > > outside of the kernel tree, so make sure that the current working > > > directory is correct and the kunit_dir is properly initialized before > > > running. > > > > > > Signed-off-by: Brendan Higgins <brendanhiggins@google.com> > > > --- > > > tools/testing/kunit/kunit.py | 8 ++++++++ > > > 1 file changed, 8 insertions(+) > > > > > > diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py > > > index 425ef40067e7..96344a11ff1f 100755 > > > --- a/tools/testing/kunit/kunit.py > > > +++ b/tools/testing/kunit/kunit.py > > > @@ -237,9 +237,14 @@ def main(argv, linux=None): > > > > > > cli_args = parser.parse_args(argv) > > > > > > + if get_kernel_root_path(): > > > + print('cd ' + get_kernel_root_path()) > > Do we want to print this, or is it a leftover debug statement? > > Whoops, I was supposed to delete that. That's embarrassing... ^_^; > > > > + os.chdir(get_kernel_root_path()) > > > + > > > if cli_args.subcommand == 'run': > > > if not os.path.exists(cli_args.build_dir): > > > os.mkdir(cli_args.build_dir) > > > + create_default_kunitconfig() > > Why are we adding this everywhere when it's already in config_tests, > > which should already be called in all of the places where a > > kunitconfig is required? > > Ah yes, .kunitconfig needs to be created before config_tests() can be > called because the LinuxSourceTree constructor needs .kunitconfig to > exist. I see. I guess the ultimate solution will be for LinuxSourceTree not require a .kunitconfig unless it's actually being used. > > Is the goal to always copy the default kunitconfig when creating a new > > build_dir? While I can sort-of see why we might want to do that, if > > the build dir doesn't exist, most of the subcommands will fail anyway > > (maybe we should only create the build-dir for 'config' and 'run'?) > > I just did it because we were getting a failure in a constructor so we > couldn't do much. Ideally we would check that the current state allows > for the command that the user intended to run, but I think that's > beyond the scope of this change. > > So I guess the real question is: Is it okay for it to crash in the > constructor with a cryptic error message for now, or do we want to let > it fail with a slightly less cryptic message later? > I personally am leaning towards allowing it to crash in the build, exec, etc. subcommands for now, and tidying up the error messages later, rather than silently creating a blank build dir, only for it then to fail later. In the meantime, yeah, we can add this for the config and run tasks, and maybe remove the whole "if cli_args.build_dir" / mkdir branch from the other subcommands. If we weren't going to fix the LinuxSourceTree constructor, it'd make sense to get rid of the redundant code to create it in config_tests(), too, but I'm not sure it's worthwhile. In any case, now I know what's happening, I'm okay with anything moderately sensible which gets the 'config' and 'run' subcommands working on an empty build dir, and the code and error messages can be fixed when tidying up the LinuxSourceTree() constructor in a separate patch. Cheers, -- David > > > if not linux: > > > linux = kunit_kernel.LinuxSourceTree() > > > @@ -257,6 +262,7 @@ def main(argv, linux=None): > > > if cli_args.build_dir: > > > if not os.path.exists(cli_args.build_dir): > > > os.mkdir(cli_args.build_dir) > > > + create_default_kunitconfig() > > > > > > if not linux: > > > linux = kunit_kernel.LinuxSourceTree() > > > @@ -273,6 +279,7 @@ def main(argv, linux=None): > > > if cli_args.build_dir: > > > if not os.path.exists(cli_args.build_dir): > > > os.mkdir(cli_args.build_dir) > > > + create_default_kunitconfig() > > > > > > if not linux: > > > linux = kunit_kernel.LinuxSourceTree() > > > @@ -291,6 +298,7 @@ def main(argv, linux=None): > > > if cli_args.build_dir: > > > if not os.path.exists(cli_args.build_dir): > > > os.mkdir(cli_args.build_dir) > > > + create_default_kunitconfig() > > > > > > if not linux: > > > linux = kunit_kernel.LinuxSourceTree() > > > > > > base-commit: 30185b69a2d533c4ba6ca926b8390ce7de495e29 > > > -- > > > 2.28.0.236.gb10cc79966-goog > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-08-11 20:00 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-08-08 1:16 [PATCH v1 1/2] kunit: tool: fix running kunit_tool from outside kernel tree Brendan Higgins 2020-08-08 1:16 ` [PATCH v1 2/2] kunit: tool: allow generating test results in JSON Brendan Higgins 2020-08-11 19:56 ` Bird, Tim 2020-08-11 20:00 ` Brendan Higgins 2020-08-08 5:45 ` [PATCH v1 1/2] kunit: tool: fix running kunit_tool from outside kernel tree David Gow 2020-08-08 8:50 ` Brendan Higgins 2020-08-11 4:29 ` David Gow
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).