[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CABVgOSnKdo1U6QFvQ3XiMoWmony93auzYyaMXs9f7UXa3GpGgg@mail.gmail.com>
Date: Sat, 6 Nov 2021 12:06:52 +0800
From: David Gow <davidgow@...gle.com>
To: Daniel Latypov <dlatypov@...gle.com>
Cc: brendanhiggins@...gle.com, linux-kernel@...r.kernel.org,
kunit-dev@...glegroups.com, linux-kselftest@...r.kernel.org,
skhan@...uxfoundation.org
Subject: Re: [PATCH 1/2] kunit: tool: move Kconfig read_from_file/parse_from_string
to package-level
On Sat, Nov 6, 2021 at 9:31 AM 'Daniel Latypov' via KUnit Development
<kunit-dev@...glegroups.com> wrote:
>
> read_from_file() clears its `self` Kconfig object and parses a config
> file.
>
> It is a way to construct Kconfig objects more so than an operation on
> Kconfig objects. This is reflected in the fact its only ever used as:
> kconfig = kunit_config.Kconfig()
> kconfig.read_from_file(path)
>
> So clean this up and simplify callers by replacing it with
> kconfig = kunit_config.parse_file(path)
>
> Do the same thing for the related parse_from_string() function as well.
>
> Signed-off-by: Daniel Latypov <dlatypov@...gle.com>
> ---
Looks sensible, works fine.
Reviewed-by: David Gow <davidgow@...gle.com>
Thanks,
-- David
> tools/testing/kunit/kunit_config.py | 61 +++++++++++++-------------
> tools/testing/kunit/kunit_kernel.py | 12 ++---
> tools/testing/kunit/kunit_tool_test.py | 6 +--
> 3 files changed, 37 insertions(+), 42 deletions(-)
>
> diff --git a/tools/testing/kunit/kunit_config.py b/tools/testing/kunit/kunit_config.py
> index c77c7d2ef622..677354546156 100644
> --- a/tools/testing/kunit/kunit_config.py
> +++ b/tools/testing/kunit/kunit_config.py
> @@ -62,33 +62,34 @@ class Kconfig(object):
> for entry in self.entries():
> f.write(str(entry) + '\n')
>
> - def parse_from_string(self, blob: str) -> None:
> - """Parses a string containing KconfigEntrys and populates this Kconfig."""
> - self._entries = []
> - is_not_set_matcher = re.compile(CONFIG_IS_NOT_SET_PATTERN)
> - config_matcher = re.compile(CONFIG_PATTERN)
> - for line in blob.split('\n'):
> - line = line.strip()
> - if not line:
> - continue
> -
> - match = config_matcher.match(line)
> - if match:
> - entry = KconfigEntry(match.group(1), match.group(2))
> - self.add_entry(entry)
> - continue
> -
> - empty_match = is_not_set_matcher.match(line)
> - if empty_match:
> - entry = KconfigEntry(empty_match.group(1), 'n')
> - self.add_entry(entry)
> - continue
> -
> - if line[0] == '#':
> - continue
> - else:
> - raise KconfigParseError('Failed to parse: ' + line)
> -
> - def read_from_file(self, path: str) -> None:
> - with open(path, 'r') as f:
> - self.parse_from_string(f.read())
> +def parse_file(path: str) -> Kconfig:
> + with open(path, 'r') as f:
> + return parse_from_string(f.read())
> +
> +def parse_from_string(blob: str) -> Kconfig:
> + """Parses a string containing Kconfig entries."""
> + kconfig = Kconfig()
> + is_not_set_matcher = re.compile(CONFIG_IS_NOT_SET_PATTERN)
> + config_matcher = re.compile(CONFIG_PATTERN)
> + for line in blob.split('\n'):
> + line = line.strip()
> + if not line:
> + continue
> +
> + match = config_matcher.match(line)
> + if match:
> + entry = KconfigEntry(match.group(1), match.group(2))
> + kconfig.add_entry(entry)
> + continue
> +
> + empty_match = is_not_set_matcher.match(line)
> + if empty_match:
> + entry = KconfigEntry(empty_match.group(1), 'n')
> + kconfig.add_entry(entry)
> + continue
> +
> + if line[0] == '#':
> + continue
> + else:
> + raise KconfigParseError('Failed to parse: ' + line)
> + return kconfig
> diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
> index 66095568bf32..51ee6e5dae91 100644
> --- a/tools/testing/kunit/kunit_kernel.py
> +++ b/tools/testing/kunit/kunit_kernel.py
> @@ -116,8 +116,7 @@ class LinuxSourceTreeOperationsQemu(LinuxSourceTreeOperations):
> self._extra_qemu_params = qemu_arch_params.extra_qemu_params
>
> def make_arch_qemuconfig(self, base_kunitconfig: kunit_config.Kconfig) -> None:
> - kconfig = kunit_config.Kconfig()
> - kconfig.parse_from_string(self._kconfig)
> + kconfig = kunit_config.parse_from_string(self._kconfig)
> base_kunitconfig.merge_in_entries(kconfig)
>
> def start(self, params: List[str], build_dir: str) -> subprocess.Popen:
> @@ -249,8 +248,7 @@ class LinuxSourceTree(object):
> if not os.path.exists(kunitconfig_path):
> shutil.copyfile(DEFAULT_KUNITCONFIG_PATH, kunitconfig_path)
>
> - self._kconfig = kunit_config.Kconfig()
> - self._kconfig.read_from_file(kunitconfig_path)
> + self._kconfig = kunit_config.parse_file(kunitconfig_path)
>
> def clean(self) -> bool:
> try:
> @@ -262,8 +260,7 @@ class LinuxSourceTree(object):
>
> def validate_config(self, build_dir) -> bool:
> kconfig_path = get_kconfig_path(build_dir)
> - validated_kconfig = kunit_config.Kconfig()
> - validated_kconfig.read_from_file(kconfig_path)
> + validated_kconfig = kunit_config.parse_file(kconfig_path)
> if not self._kconfig.is_subset_of(validated_kconfig):
> invalid = self._kconfig.entries() - validated_kconfig.entries()
> message = 'Provided Kconfig is not contained in validated .config. Following fields found in kunitconfig, ' \
> @@ -291,8 +288,7 @@ class LinuxSourceTree(object):
> """Creates a new .config if it is not a subset of the .kunitconfig."""
> kconfig_path = get_kconfig_path(build_dir)
> if os.path.exists(kconfig_path):
> - existing_kconfig = kunit_config.Kconfig()
> - existing_kconfig.read_from_file(kconfig_path)
> + existing_kconfig = kunit_config.parse_file(kconfig_path)
> self._ops.make_arch_qemuconfig(self._kconfig)
> if not self._kconfig.is_subset_of(existing_kconfig):
> print('Regenerating .config ...')
> diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
> index 9c4126731457..4ec70e41ec5a 100755
> --- a/tools/testing/kunit/kunit_tool_test.py
> +++ b/tools/testing/kunit/kunit_tool_test.py
> @@ -50,10 +50,9 @@ class KconfigTest(unittest.TestCase):
> self.assertFalse(kconfig1.is_subset_of(kconfig0))
>
> def test_read_from_file(self):
> - kconfig = kunit_config.Kconfig()
> kconfig_path = test_data_path('test_read_from_file.kconfig')
>
> - kconfig.read_from_file(kconfig_path)
> + kconfig = kunit_config.parse_file(kconfig_path)
>
> expected_kconfig = kunit_config.Kconfig()
> expected_kconfig.add_entry(
> @@ -86,8 +85,7 @@ class KconfigTest(unittest.TestCase):
>
> expected_kconfig.write_to_file(kconfig_path)
>
> - actual_kconfig = kunit_config.Kconfig()
> - actual_kconfig.read_from_file(kconfig_path)
> + actual_kconfig = kunit_config.parse_file(kconfig_path)
>
> self.assertEqual(actual_kconfig.entries(),
> expected_kconfig.entries())
>
> base-commit: 52a5d80a2225e2d0b2a8f4656b76aead2a443b2a
> --
> 2.34.0.rc0.344.g81b53c2807-goog
>
> --
> You received this message because you are subscribed to the Google Groups "KUnit Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@...glegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20211106013058.2621799-1-dlatypov%40google.com.
Powered by blists - more mailing lists