From: SeongJae Park sjpark@amazon.de
Because commit d43c7fb05765 ("kunit: tool: fix improper treatment of file location") removed 'kunit_kernel.kunitconfig_path' modification for the '--builddir' argument, running kunit with '--build_dir' now fails with below error message:
Traceback (most recent call last): File "./tools/testing/kunit/kunit.py", line 325, in <module> main(sys.argv[1:]) File "./tools/testing/kunit/kunit.py", line 245, in main linux = kunit_kernel.LinuxSourceTree() File "/home/sjpark/linux/tools/testing/kunit/kunit_kernel.py", line 109, in __init__ self._kconfig.read_from_file(kunitconfig_path) File "/home/sjpark/linux/tools/testing/kunit/kunit_config.py", line 88, in read_from_file with open(path, 'r') as f: FileNotFoundError: [Errno 2] No such file or directory: '.kunitconfig'
As simply reverting the change now make the 'kunit_tool_test' fails again, this commit fixes the problem by passing the 'build_dir' argument to 'LinuxSourceTree' constructor.
Fixes: d43c7fb05765 ("kunit: tool: fix improper treatment of file location") Signed-off-by: SeongJae Park sjpark@amazon.de --- tools/testing/kunit/kunit.py | 8 ++++---- tools/testing/kunit/kunit_kernel.py | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py index 425ef40067e7..611c23e178f8 100755 --- a/tools/testing/kunit/kunit.py +++ b/tools/testing/kunit/kunit.py @@ -242,7 +242,7 @@ def main(argv, linux=None): os.mkdir(cli_args.build_dir)
if not linux: - linux = kunit_kernel.LinuxSourceTree() + linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir)
request = KunitRequest(cli_args.raw_output, cli_args.timeout, @@ -259,7 +259,7 @@ def main(argv, linux=None): os.mkdir(cli_args.build_dir)
if not linux: - linux = kunit_kernel.LinuxSourceTree() + linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir)
request = KunitConfigRequest(cli_args.build_dir, cli_args.make_options) @@ -275,7 +275,7 @@ def main(argv, linux=None): os.mkdir(cli_args.build_dir)
if not linux: - linux = kunit_kernel.LinuxSourceTree() + linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir)
request = KunitBuildRequest(cli_args.jobs, cli_args.build_dir, @@ -293,7 +293,7 @@ def main(argv, linux=None): os.mkdir(cli_args.build_dir)
if not linux: - linux = kunit_kernel.LinuxSourceTree() + linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir)
exec_request = KunitExecRequest(cli_args.timeout, cli_args.build_dir, diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py index e20e2056cb38..16a997504317 100644 --- a/tools/testing/kunit/kunit_kernel.py +++ b/tools/testing/kunit/kunit_kernel.py @@ -104,9 +104,9 @@ def get_kconfig_path(build_dir): class LinuxSourceTree(object): """Represents a Linux kernel source tree with KUnit tests."""
- def __init__(self): + def __init__(self, build_dir): self._kconfig = kunit_config.Kconfig() - self._kconfig.read_from_file(kunitconfig_path) + self._kconfig.read_from_file(os.path.join(build_dir, kunitconfig_path)) self._ops = LinuxSourceTreeOperations() signal.signal(signal.SIGINT, self.signal_handler)
From: SeongJae Park sjpark@amazon.de
'kunit_kernel.kunittest_config' was constant at first, and therefore it used UPPER_SNAKE_CASE naming convention that usually means it is constant in Python world. But, commit e3212513a8f0 ("kunit: Create default config in '--build_dir'") made it modifiable to fix a use case of the tool and thus the naming also changed to lower_snake_case. However, this resulted in a confusion. As a result, some successing changes made the tool unittest fail, and a fix[1] of it again incurred the '--build_dir' use case failure.
As the previous commit fixed the '--build_dir' use case without modifying the variable again, this commit marks the variable as constant again with UPPER_SNAKE_CASE, to reduce future confusions.
[1] Commit d43c7fb05765 ("kunit: tool: fix improper treatment of file location")
Signed-off-by: SeongJae Park sjpark@amazon.de --- tools/testing/kunit/kunit.py | 4 ++-- tools/testing/kunit/kunit_kernel.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py index 611c23e178f8..0a58c1fb87d9 100755 --- a/tools/testing/kunit/kunit.py +++ b/tools/testing/kunit/kunit.py @@ -44,9 +44,9 @@ class KunitStatus(Enum): TEST_FAILURE = auto()
def create_default_kunitconfig(): - if not os.path.exists(kunit_kernel.kunitconfig_path): + if not os.path.exists(kunit_kernel.KUNITCONFIG_PATH): shutil.copyfile('arch/um/configs/kunit_defconfig', - kunit_kernel.kunitconfig_path) + kunit_kernel.KUNITCONFIG_PATH)
def get_kernel_root_path(): parts = sys.argv[0] if not __file__ else __file__ diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py index 16a997504317..42dca0163479 100644 --- a/tools/testing/kunit/kunit_kernel.py +++ b/tools/testing/kunit/kunit_kernel.py @@ -18,7 +18,7 @@ import kunit_config import kunit_parser
KCONFIG_PATH = '.config' -kunitconfig_path = '.kunitconfig' +KUNITCONFIG_PATH = '.kunitconfig' BROKEN_ALLCONFIG_PATH = 'tools/testing/kunit/configs/broken_on_uml.config'
class ConfigError(Exception): @@ -106,7 +106,7 @@ class LinuxSourceTree(object):
def __init__(self, build_dir): self._kconfig = kunit_config.Kconfig() - self._kconfig.read_from_file(os.path.join(build_dir, kunitconfig_path)) + self._kconfig.read_from_file(os.path.join(build_dir, KUNITCONFIG_PATH)) self._ops = LinuxSourceTreeOperations() signal.signal(signal.SIGINT, self.signal_handler)
ping
On Mon, 12 Oct 2020 12:26:21 +0200 SeongJae Park sjpark@amazon.com wrote:
From: SeongJae Park sjpark@amazon.de
'kunit_kernel.kunittest_config' was constant at first, and therefore it used UPPER_SNAKE_CASE naming convention that usually means it is constant in Python world. But, commit e3212513a8f0 ("kunit: Create default config in '--build_dir'") made it modifiable to fix a use case of the tool and thus the naming also changed to lower_snake_case. However, this resulted in a confusion. As a result, some successing changes made the tool unittest fail, and a fix[1] of it again incurred the '--build_dir' use case failure.
As the previous commit fixed the '--build_dir' use case without modifying the variable again, this commit marks the variable as constant again with UPPER_SNAKE_CASE, to reduce future confusions.
[1] Commit d43c7fb05765 ("kunit: tool: fix improper treatment of file location")
Signed-off-by: SeongJae Park sjpark@amazon.de
tools/testing/kunit/kunit.py | 4 ++-- tools/testing/kunit/kunit_kernel.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py index 611c23e178f8..0a58c1fb87d9 100755 --- a/tools/testing/kunit/kunit.py +++ b/tools/testing/kunit/kunit.py @@ -44,9 +44,9 @@ class KunitStatus(Enum): TEST_FAILURE = auto() def create_default_kunitconfig():
- if not os.path.exists(kunit_kernel.kunitconfig_path):
- if not os.path.exists(kunit_kernel.KUNITCONFIG_PATH): shutil.copyfile('arch/um/configs/kunit_defconfig',
kunit_kernel.kunitconfig_path)
kunit_kernel.KUNITCONFIG_PATH)def get_kernel_root_path(): parts = sys.argv[0] if not __file__ else __file__ diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py index 16a997504317..42dca0163479 100644 --- a/tools/testing/kunit/kunit_kernel.py +++ b/tools/testing/kunit/kunit_kernel.py @@ -18,7 +18,7 @@ import kunit_config import kunit_parser KCONFIG_PATH = '.config' -kunitconfig_path = '.kunitconfig' +KUNITCONFIG_PATH = '.kunitconfig' BROKEN_ALLCONFIG_PATH = 'tools/testing/kunit/configs/broken_on_uml.config' class ConfigError(Exception): @@ -106,7 +106,7 @@ class LinuxSourceTree(object): def __init__(self, build_dir): self._kconfig = kunit_config.Kconfig()
self._kconfig.read_from_file(os.path.join(build_dir, kunitconfig_path))
self._kconfig.read_from_file(os.path.join(build_dir, KUNITCONFIG_PATH))2.17.1
On Mon, Oct 12, 2020 at 3:27 AM SeongJae Park sjpark@amazon.com wrote:
From: SeongJae Park sjpark@amazon.de
'kunit_kernel.kunittest_config' was constant at first, and therefore it used UPPER_SNAKE_CASE naming convention that usually means it is constant in Python world. But, commit e3212513a8f0 ("kunit: Create default config in '--build_dir'") made it modifiable to fix a use case of the tool and thus the naming also changed to lower_snake_case. However, this resulted in a confusion. As a result, some successing changes made the tool unittest fail, and a fix[1] of it again incurred the '--build_dir' use case failure.
As the previous commit fixed the '--build_dir' use case without modifying the variable again, this commit marks the variable as constant again with UPPER_SNAKE_CASE, to reduce future confusions.
[1] Commit d43c7fb05765 ("kunit: tool: fix improper treatment of file location")
Signed-off-by: SeongJae Park sjpark@amazon.de
Reviewed-by: Brendan Higgins brendanhiggins@google.com
Thanks for this! This is something I meant to fix a while ago and forgot about.
One minor issue, this patch does not apply on torvalds/master right now. Could you please rebase this?
On Wed, 21 Oct 2020 14:32:52 -0700 Brendan Higgins brendanhiggins@google.com wrote:
On Mon, Oct 12, 2020 at 3:27 AM SeongJae Park sjpark@amazon.com wrote:
From: SeongJae Park sjpark@amazon.de
'kunit_kernel.kunittest_config' was constant at first, and therefore it used UPPER_SNAKE_CASE naming convention that usually means it is constant in Python world. But, commit e3212513a8f0 ("kunit: Create default config in '--build_dir'") made it modifiable to fix a use case of the tool and thus the naming also changed to lower_snake_case. However, this resulted in a confusion. As a result, some successing changes made the tool unittest fail, and a fix[1] of it again incurred the '--build_dir' use case failure.
As the previous commit fixed the '--build_dir' use case without modifying the variable again, this commit marks the variable as constant again with UPPER_SNAKE_CASE, to reduce future confusions.
[1] Commit d43c7fb05765 ("kunit: tool: fix improper treatment of file location")
Signed-off-by: SeongJae Park sjpark@amazon.de
Reviewed-by: Brendan Higgins brendanhiggins@google.com
Thanks :)
Thanks for this! This is something I meant to fix a while ago and forgot about.
One minor issue, this patch does not apply on torvalds/master right now. Could you please rebase this?
Surely of course, I will send next version soon.
Thanks, SeongJae Park
On Thu, Oct 22, 2020 at 08:35:26AM +0200, SeongJae Park wrote:
On Wed, 21 Oct 2020 14:32:52 -0700 Brendan Higgins brendanhiggins@google.com wrote:
On Mon, Oct 12, 2020 at 3:27 AM SeongJae Park sjpark@amazon.com wrote:
From: SeongJae Park sjpark@amazon.de
'kunit_kernel.kunittest_config' was constant at first, and therefore it used UPPER_SNAKE_CASE naming convention that usually means it is constant in Python world. But, commit e3212513a8f0 ("kunit: Create default config in '--build_dir'") made it modifiable to fix a use case of the tool and thus the naming also changed to lower_snake_case. However, this resulted in a confusion. As a result, some successing changes made the tool unittest fail, and a fix[1] of it again incurred the '--build_dir' use case failure.
As the previous commit fixed the '--build_dir' use case without modifying the variable again, this commit marks the variable as constant again with UPPER_SNAKE_CASE, to reduce future confusions.
[1] Commit d43c7fb05765 ("kunit: tool: fix improper treatment of file location")
Signed-off-by: SeongJae Park sjpark@amazon.de
Reviewed-by: Brendan Higgins brendanhiggins@google.com
Thanks :)
Thanks for this! This is something I meant to fix a while ago and forgot about.
One minor issue, this patch does not apply on torvalds/master right now. Could you please rebase this?
Surely of course, I will send next version soon.
May I ask what happened to [1]? I mean it seems these two are goind to collide.
Brendan?
[1]: https://lore.kernel.org/linux-kselftest/20201015152348.65147-1-andriy.shevch...
On Sun, Oct 25, 2020 at 5:45 AM andy@surfacebook.localdomain wrote:
On Thu, Oct 22, 2020 at 08:35:26AM +0200, SeongJae Park wrote:
On Wed, 21 Oct 2020 14:32:52 -0700 Brendan Higgins brendanhiggins@google.com wrote:
On Mon, Oct 12, 2020 at 3:27 AM SeongJae Park sjpark@amazon.com wrote:
From: SeongJae Park sjpark@amazon.de
'kunit_kernel.kunittest_config' was constant at first, and therefore it used UPPER_SNAKE_CASE naming convention that usually means it is constant in Python world. But, commit e3212513a8f0 ("kunit: Create default config in '--build_dir'") made it modifiable to fix a use case of the tool and thus the naming also changed to lower_snake_case. However, this resulted in a confusion. As a result, some successing changes made the tool unittest fail, and a fix[1] of it again incurred the '--build_dir' use case failure.
As the previous commit fixed the '--build_dir' use case without modifying the variable again, this commit marks the variable as constant again with UPPER_SNAKE_CASE, to reduce future confusions.
[1] Commit d43c7fb05765 ("kunit: tool: fix improper treatment of file location")
Signed-off-by: SeongJae Park sjpark@amazon.de
Reviewed-by: Brendan Higgins brendanhiggins@google.com
Thanks :)
Thanks for this! This is something I meant to fix a while ago and forgot about.
One minor issue, this patch does not apply on torvalds/master right now. Could you please rebase this?
Surely of course, I will send next version soon.
May I ask what happened to [1]? I mean it seems these two are goind to collide.
Brendan?
Sorry for the confusion here. After an initial glance at your patches (before I did the review end of last week) I thought only the first patch from SeongJae would potentially conflict with yours (Andy's) (hence why I hadn't reviewed it yet, I was waiting until after I looked at yours).
I noticed on Friday that SeongJae's changes were actually fully encompassed by Andy's, so I am taking Andy's not SongJae's. Sorry, I was going to notify SongJae today, but you beat me to it.
Sorry everyone.
On Mon, 26 Oct 2020 13:44:33 -0700 Brendan Higgins brendanhiggins@google.com wrote:
On Sun, Oct 25, 2020 at 5:45 AM andy@surfacebook.localdomain wrote:
On Thu, Oct 22, 2020 at 08:35:26AM +0200, SeongJae Park wrote:
On Wed, 21 Oct 2020 14:32:52 -0700 Brendan Higgins brendanhiggins@google.com wrote:
On Mon, Oct 12, 2020 at 3:27 AM SeongJae Park sjpark@amazon.com wrote:
From: SeongJae Park sjpark@amazon.de
'kunit_kernel.kunittest_config' was constant at first, and therefore it used UPPER_SNAKE_CASE naming convention that usually means it is constant in Python world. But, commit e3212513a8f0 ("kunit: Create default config in '--build_dir'") made it modifiable to fix a use case of the tool and thus the naming also changed to lower_snake_case. However, this resulted in a confusion. As a result, some successing changes made the tool unittest fail, and a fix[1] of it again incurred the '--build_dir' use case failure.
As the previous commit fixed the '--build_dir' use case without modifying the variable again, this commit marks the variable as constant again with UPPER_SNAKE_CASE, to reduce future confusions.
[1] Commit d43c7fb05765 ("kunit: tool: fix improper treatment of file location")
Signed-off-by: SeongJae Park sjpark@amazon.de
Reviewed-by: Brendan Higgins brendanhiggins@google.com
Thanks :)
Thanks for this! This is something I meant to fix a while ago and forgot about.
One minor issue, this patch does not apply on torvalds/master right now. Could you please rebase this?
Surely of course, I will send next version soon.
May I ask what happened to [1]? I mean it seems these two are goind to collide.
Brendan?
Sorry for the confusion here. After an initial glance at your patches (before I did the review end of last week) I thought only the first patch from SeongJae would potentially conflict with yours (Andy's) (hence why I hadn't reviewed it yet, I was waiting until after I looked at yours).
I noticed on Friday that SeongJae's changes were actually fully encompassed by Andy's, so I am taking Andy's not SongJae's. Sorry, I was going to notify SongJae today, but you beat me to it.
Sorry everyone.
It's ok, I understand the situation and respect your decision. After all, what I really wanted was just fixing the problem by whoever. I would like to say thank you to Andy for fixing it instead of me :) Also, thank you Brendan, for maintaining the cool Kunit ;)
Thanks, SeongJae Park
linux-kselftest-mirror@lists.linaro.org
-
andy＠surfacebook.localdomain -
Brendan Higgins -
SeongJae Park