| Message ID | 20220520224200.3764027-1-dlatypov@google.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Brendan Higgins |
| Headers | show |
| Series | [v2] kunit: tool: refactor internal kconfig handling, allow overriding | expand |
On Sat, May 21, 2022 at 6:42 AM Daniel Latypov <dlatypov@google.com> wrote: > > Currently, you cannot ovewrwrite what's in your kunitconfig via > --kconfig_add. > Nor can you override something in a qemu_config via either means. > > This patch makes it so we have this level of priority > * --kconfig_add > * kunitconfig file (the default or the one from --kunitconfig) > * qemu_config > > The rationale for this order is that the more "dynamic" sources of > kconfig options should take priority. > > --kconfig_add is obviously the most dynamic. > And for kunitconfig, users probably tweak the file manually or specify > --kunitconfig more often than they delve into qemu_config python files. > > And internally, we convert the kconfigs from a python list into a set or > dict fairly often. We should just use a dict internally. > We exposed the set transform in the past since we didn't define __eq__, > so also take the chance to shore up the kunit_kconfig.Kconfig interface. > > Example > ======= > > Let's consider the unrealistic example where someone would want to > disable CONFIG_KUNIT. > I.e. they run > $ ./tools/testing/kunit/kunit.py config --kconfig_add=CONFIG_KUNIT=n > > Before > ------ > We'd write the following > > # CONFIG_KUNIT is not set > > CONFIG_KUNIT_ALL_TESTS=y > > CONFIG_KUNIT_TEST=y > > CONFIG_KUNIT=y > > CONFIG_KUNIT_EXAMPLE_TEST=y > > And we'd error out with > > ERROR:root:Not all Kconfig options selected in kunitconfig were in the generated .config. > > This is probably due to unsatisfied dependencies. > > Missing: # CONFIG_KUNIT is not set > > After > ----- > We'd write the following > > # CONFIG_KUNIT is not set > > CONFIG_KUNIT_TEST=y > > CONFIG_KUNIT_ALL_TESTS=y > > CONFIG_KUNIT_EXAMPLE_TEST=y > > And we'd error out with > > ERROR:root:Not all Kconfig options selected in kunitconfig were in the generated .config. > > This is probably due to unsatisfied dependencies. > > Missing: CONFIG_KUNIT_EXAMPLE_TEST=y, CONFIG_KUNIT_TEST=y, CONFIG_KUNIT_ALL_TESTS=y > > Signed-off-by: Daniel Latypov <dlatypov@google.com> > --- > v1 -> v2: fix validate_config() func. > There was a bug found by David, see > https://lore.kernel.org/linux-kselftest/CAGS_qxpF338dvbB+6QW1n8_agddeS10+nkTQNmT+0UcvoE1gBw@mail.gmail.com/ > --- Sorry for the delay, finally getting around to reviewing this version. It looks good to me, and works well enough in testing that I've got no real complaints. A couple of minor comments below, but nothing actually worth doing a new version for, IMO. Reviewed-by: David Gow <davidgow@google.com> Cheers, -- David > tools/testing/kunit/kunit_config.py | 49 +++++++++++++++----------- > tools/testing/kunit/kunit_kernel.py | 18 +++++----- > tools/testing/kunit/kunit_tool_test.py | 45 ++++++++++------------- > 3 files changed, 57 insertions(+), 55 deletions(-) > > diff --git a/tools/testing/kunit/kunit_config.py b/tools/testing/kunit/kunit_config.py > index 75a8dc1683d4..89443400b17e 100644 > --- a/tools/testing/kunit/kunit_config.py > +++ b/tools/testing/kunit/kunit_config.py > @@ -8,7 +8,7 @@ > > from dataclasses import dataclass > import re > -from typing import List, Set > +from typing import Dict, Iterable, Set > > CONFIG_IS_NOT_SET_PATTERN = r'^# CONFIG_(\w+) is not set$' > CONFIG_PATTERN = r'^CONFIG_(\w+)=(\S+|".*")$' > @@ -32,35 +32,46 @@ class Kconfig: > """Represents defconfig or .config specified using the Kconfig language.""" > > def __init__(self) -> None: > - self._entries = [] # type: List[KconfigEntry] > + self._entries = {} # type: Dict[str, str] > > - def entries(self) -> Set[KconfigEntry]: > - return set(self._entries) > + def __eq__(self, other) -> bool: > + if not isinstance(other, self.__class__): > + return False > + return self._entries == other._entries > > - def add_entry(self, entry: KconfigEntry) -> None: > - self._entries.append(entry) > + def __repr__(self) -> str: > + return ','.join(str(e) for e in self._as_entries()) > + > + > + def _as_entries(self) -> Iterable[KconfigEntry]: > + for name, value in self._entries.items(): > + yield KconfigEntry(name, value) > + > + def add_entry(self, name: str, value: str) -> None: > + self._entries[name] = value > > def is_subset_of(self, other: 'Kconfig') -> bool: > - other_dict = {e.name: e.value for e in other.entries()} > - for a in self.entries(): > - b = other_dict.get(a.name) > + for name, value in self._entries.items(): > + b = other._entries.get(name) > if b is None: > - if a.value == 'n': > + if value == 'n': > continue > return False > - if a.value != b: > + if value != b: > return False > return True > > + def set_diff(self, other: 'Kconfig') -> Set[KconfigEntry]: > + return set(self._as_entries()) - set(other._as_entries()) > + It took me a couple of goes to realise that this was looking at the difference between sets, not trying to set the difference. Maybe different_entries() or something like that'd be clearer, but I can't say it bothers me enough to be worth a new version. Then again (as noted below), the direct set difference isn't exactly what we want, it's more the equivalent of is_subset_of(). The follow-up repeated-kunitconfig patch adds differing_options(), which is closer to what we'd want, I think: https://lore.kernel.org/linux-kselftest/20220624001247.3255978-1-dlatypov@google.com/ Probably easiest to accept this patch as-is, followed by those follow-up ones, and adjust it then, if that's worth doing, though... > def merge_in_entries(self, other: 'Kconfig') -> None: > - if other.is_subset_of(self): > - return > - self._entries = list(self.entries().union(other.entries())) > + for name, value in other._entries.items(): > + self._entries[name] = value > > def write_to_file(self, path: str) -> None: > with open(path, 'a+') as f: > - for entry in self.entries(): > - f.write(str(entry) + '\n') > + for e in self._as_entries(): > + f.write(str(e) + '\n') > > def parse_file(path: str) -> Kconfig: > with open(path, 'r') as f: > @@ -78,14 +89,12 @@ def parse_from_string(blob: str) -> Kconfig: > > match = config_matcher.match(line) > if match: > - entry = KconfigEntry(match.group(1), match.group(2)) > - kconfig.add_entry(entry) > + kconfig.add_entry(match.group(1), match.group(2)) > continue > > empty_match = is_not_set_matcher.match(line) > if empty_match: > - entry = KconfigEntry(empty_match.group(1), 'n') > - kconfig.add_entry(entry) > + kconfig.add_entry(empty_match.group(1), 'n') > continue > > if line[0] == '#': > diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py > index 3539efaf99ba..6d994bb24999 100644 > --- a/tools/testing/kunit/kunit_kernel.py > +++ b/tools/testing/kunit/kunit_kernel.py > @@ -53,8 +53,8 @@ class LinuxSourceTreeOperations: > except subprocess.CalledProcessError as e: > raise ConfigError(e.output.decode()) > > - def make_arch_qemuconfig(self, base_kunitconfig: kunit_config.Kconfig) -> None: > - pass > + def make_arch_qemuconfig(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig: > + return base_kunitconfig > > def make_allyesconfig(self, build_dir: str, make_options) -> None: > raise ConfigError('Only the "um" arch is supported for alltests') > @@ -109,9 +109,10 @@ class LinuxSourceTreeOperationsQemu(LinuxSourceTreeOperations): > self._kernel_command_line = qemu_arch_params.kernel_command_line + ' kunit_shutdown=reboot' > self._extra_qemu_params = qemu_arch_params.extra_qemu_params > > - def make_arch_qemuconfig(self, base_kunitconfig: kunit_config.Kconfig) -> None: > + def make_arch_qemuconfig(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig: > kconfig = kunit_config.parse_from_string(self._kconfig) > - base_kunitconfig.merge_in_entries(kconfig) > + kconfig.merge_in_entries(base_kunitconfig) > + return kconfig > > def start(self, params: List[str], build_dir: str) -> subprocess.Popen: > kernel_path = os.path.join(build_dir, self._kernel_path) > @@ -267,7 +268,7 @@ class LinuxSourceTree: > validated_kconfig = kunit_config.parse_file(kconfig_path) > if self._kconfig.is_subset_of(validated_kconfig): > return True > - invalid = self._kconfig.entries() - validated_kconfig.entries() > + invalid = self._kconfig.set_diff(validated_kconfig) The fact that the set 'invalid' is not actually equal to the set of conflicting entries is_subset_of() finds bothers me slightly, though due to the early return, I don't think it should ever be a problem in pracitce. Maybe having a 'subset_of' function, instead of set_diff, which does the whole "not set" is equivalent to "n" check would be better? Not worth changing this now, though: let's leave anything too heroic for the next bout of kconfig-related stuff. (For example, there are plans afoot to actually user CONFIG_x=n instead of is not set in the files: https://lore.kernel.org/lkml/CA+icZUXkd=dtbBX3UKLRzGiVSKC=TeW7ATiRKD9dnYtmm6RZqg@mail.gmail.com/T/ ) > message = 'Not all Kconfig options selected in kunitconfig were in the generated .config.\n' \ > 'This is probably due to unsatisfied dependencies.\n' \ > 'Missing: ' + ', '.join([str(e) for e in invalid]) > @@ -282,7 +283,7 @@ class LinuxSourceTree: > if build_dir and not os.path.exists(build_dir): > os.mkdir(build_dir) > try: > - self._ops.make_arch_qemuconfig(self._kconfig) > + self._kconfig = self._ops.make_arch_qemuconfig(self._kconfig) > self._kconfig.write_to_file(kconfig_path) > self._ops.make_olddefconfig(build_dir, make_options) > except ConfigError as e: > @@ -303,7 +304,7 @@ class LinuxSourceTree: > return True > > old_kconfig = kunit_config.parse_file(old_path) > - return old_kconfig.entries() != self._kconfig.entries() > + return old_kconfig != self._kconfig > > def build_reconfig(self, build_dir: str, make_options) -> bool: > """Creates a new .config if it is not a subset of the .kunitconfig.""" > @@ -313,7 +314,8 @@ class LinuxSourceTree: > return self.build_config(build_dir, make_options) > > existing_kconfig = kunit_config.parse_file(kconfig_path) > - self._ops.make_arch_qemuconfig(self._kconfig) > + self._kconfig = self._ops.make_arch_qemuconfig(self._kconfig) > + > if self._kconfig.is_subset_of(existing_kconfig) and not self._kunitconfig_changed(build_dir): > return True > print('Regenerating .config ...') > diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py > index 25a2eb3bf114..3a8f638ff092 100755 > --- a/tools/testing/kunit/kunit_tool_test.py > +++ b/tools/testing/kunit/kunit_tool_test.py > @@ -45,7 +45,7 @@ class KconfigTest(unittest.TestCase): > self.assertTrue(kconfig0.is_subset_of(kconfig0)) > > kconfig1 = kunit_config.Kconfig() > - kconfig1.add_entry(kunit_config.KconfigEntry('TEST', 'y')) > + kconfig1.add_entry('TEST', 'y') > self.assertTrue(kconfig1.is_subset_of(kconfig1)) > self.assertTrue(kconfig0.is_subset_of(kconfig1)) > self.assertFalse(kconfig1.is_subset_of(kconfig0)) > @@ -56,40 +56,28 @@ class KconfigTest(unittest.TestCase): > kconfig = kunit_config.parse_file(kconfig_path) > > expected_kconfig = kunit_config.Kconfig() > - expected_kconfig.add_entry( > - kunit_config.KconfigEntry('UML', 'y')) > - expected_kconfig.add_entry( > - kunit_config.KconfigEntry('MMU', 'y')) > - expected_kconfig.add_entry( > - kunit_config.KconfigEntry('TEST', 'y')) > - expected_kconfig.add_entry( > - kunit_config.KconfigEntry('EXAMPLE_TEST', 'y')) > - expected_kconfig.add_entry( > - kunit_config.KconfigEntry('MK8', 'n')) > - > - self.assertEqual(kconfig.entries(), expected_kconfig.entries()) > + expected_kconfig.add_entry('UML', 'y') > + expected_kconfig.add_entry('MMU', 'y') > + expected_kconfig.add_entry('TEST', 'y') > + expected_kconfig.add_entry('EXAMPLE_TEST', 'y') > + expected_kconfig.add_entry('MK8', 'n') > + > + self.assertEqual(kconfig, expected_kconfig) > > def test_write_to_file(self): > kconfig_path = os.path.join(test_tmpdir, '.config') > > expected_kconfig = kunit_config.Kconfig() > - expected_kconfig.add_entry( > - kunit_config.KconfigEntry('UML', 'y')) > - expected_kconfig.add_entry( > - kunit_config.KconfigEntry('MMU', 'y')) > - expected_kconfig.add_entry( > - kunit_config.KconfigEntry('TEST', 'y')) > - expected_kconfig.add_entry( > - kunit_config.KconfigEntry('EXAMPLE_TEST', 'y')) > - expected_kconfig.add_entry( > - kunit_config.KconfigEntry('MK8', 'n')) > + expected_kconfig.add_entry('UML', 'y') > + expected_kconfig.add_entry('MMU', 'y') > + expected_kconfig.add_entry('TEST', 'y') > + expected_kconfig.add_entry('EXAMPLE_TEST', 'y') > + expected_kconfig.add_entry('MK8', 'n') > > expected_kconfig.write_to_file(kconfig_path) > > actual_kconfig = kunit_config.parse_file(kconfig_path) > - > - self.assertEqual(actual_kconfig.entries(), > - expected_kconfig.entries()) > + self.assertEqual(actual_kconfig, expected_kconfig) > > class KUnitParserTest(unittest.TestCase): > > @@ -381,8 +369,11 @@ class LinuxSourceTreeTest(unittest.TestCase): > kunit_kernel.LinuxSourceTree('', kunitconfig_path=dir) > > def test_kconfig_add(self): > + want_kconfig = kunit_config.Kconfig() > + want_kconfig.add_entry('NOT_REAL', 'y') > + > tree = kunit_kernel.LinuxSourceTree('', kconfig_add=['CONFIG_NOT_REAL=y']) > - self.assertIn(kunit_config.KconfigEntry('NOT_REAL', 'y'), tree._kconfig.entries()) > + self.assertFalse(want_kconfig.set_diff(tree._kconfig)) > > def test_invalid_arch(self): > with self.assertRaisesRegex(kunit_kernel.ConfigError, 'not a valid arch, options are.*x86_64'): > > base-commit: 1b11063d32d7e11366e48be64215ff517ce32217 > -- > 2.36.1.124.g0e6072fb45-goog >
On Fri, Jun 24, 2022 at 12:55 AM David Gow <davidgow@google.com> wrote: > > > > + def set_diff(self, other: 'Kconfig') -> Set[KconfigEntry]: > > + return set(self._as_entries()) - set(other._as_entries()) > > + > > It took me a couple of goes to realise that this was looking at the > difference between sets, not trying to set the difference. Maybe > different_entries() or something like that'd be clearer, but I can't > say it bothers me enough to be worth a new version. Wdyt about `set_difference()`? Or maybe adding a verb: `get_set_diff()`, `compute_set_diff()`? But we do want to make it clear it has set (asymmetric) difference semantics, see below. > > Then again (as noted below), the direct set difference isn't exactly > what we want, it's more the equivalent of is_subset_of(). The > follow-up repeated-kunitconfig patch adds differing_options(), which > is closer to what we'd want, I think: > https://lore.kernel.org/linux-kselftest/20220624001247.3255978-1-dlatypov@google.com/ differing_options() does not have the right semantics. For this, we do explicitly want a set difference. Context: This is the func used to print out these warnings: $ .../kunit.py run --kconfig_add=CONFIG_PCI=y ... Missing: CONFIG_PCI=y That comes from invalid = self._kconfig.set_diff(validated_kconfig) Using differing_options() to get our version of the configs: invalid = (want for want, got in self._kconfig.differing_options(validated_kconfig)) we instead get an empty list. The problem is that differing_options() only shows config options that are explicitly to different values. There's probably a way I can name these functions better to make the difference more clear. Or perhaps we should move set_diff() out of this class and have kunit_kernel.py itself due the computation. E.g. // make as_entries() "public", s/invalid/missing missing = set(self._kconfig.as_entries()) - set(validated_config.as_entries()) Thoughts? Daniel
On Sat, Jun 25, 2022 at 4:24 AM Daniel Latypov <dlatypov@google.com> wrote: > > On Fri, Jun 24, 2022 at 12:55 AM David Gow <davidgow@google.com> wrote: > > > > > > + def set_diff(self, other: 'Kconfig') -> Set[KconfigEntry]: > > > + return set(self._as_entries()) - set(other._as_entries()) > > > + > > > > It took me a couple of goes to realise that this was looking at the > > difference between sets, not trying to set the difference. Maybe > > different_entries() or something like that'd be clearer, but I can't > > say it bothers me enough to be worth a new version. > > Wdyt about `set_difference()`? > Or maybe adding a verb: `get_set_diff()`, `compute_set_diff()`? > > But we do want to make it clear it has set (asymmetric) difference > semantics, see below. > I like adding a verb, personally. > > > > Then again (as noted below), the direct set difference isn't exactly > > what we want, it's more the equivalent of is_subset_of(). The > > follow-up repeated-kunitconfig patch adds differing_options(), which > > is closer to what we'd want, I think: > > https://lore.kernel.org/linux-kselftest/20220624001247.3255978-1-dlatypov@google.com/ > > differing_options() does not have the right semantics. > For this, we do explicitly want a set difference. > > Context: This is the func used to print out these warnings: > $ .../kunit.py run --kconfig_add=CONFIG_PCI=y > ... > Missing: CONFIG_PCI=y > > That comes from > invalid = self._kconfig.set_diff(validated_kconfig) > Using differing_options() to get our version of the configs: > invalid = (want for want, got in > self._kconfig.differing_options(validated_kconfig)) > we instead get an empty list. > > The problem is that differing_options() only shows config options that > are explicitly to different values. Ah, yep: my mistake. I was mixing up the (!b && value=n) check in is_subset_of with the (b && value != b) in differing options. Whoops. But, thinking about it, the special handling of "=n" in set A not needing an equivalent in set B should fall out from the set difference anyway. So no problem here at all. > There's probably a way I can name these functions better to make the > difference more clear. How about "conflicting_options" versus "missing_options"? > Or perhaps we should move set_diff() out of this class and have > kunit_kernel.py itself due the computation. > > E.g. > // make as_entries() "public", s/invalid/missing > missing = set(self._kconfig.as_entries()) - set(validated_config.as_entries()) > > Thoughts? Fair enough point: > > Daniel
diff --git a/tools/testing/kunit/kunit_config.py b/tools/testing/kunit/kunit_config.py index 75a8dc1683d4..89443400b17e 100644 --- a/tools/testing/kunit/kunit_config.py +++ b/tools/testing/kunit/kunit_config.py @@ -8,7 +8,7 @@ from dataclasses import dataclass import re -from typing import List, Set +from typing import Dict, Iterable, Set CONFIG_IS_NOT_SET_PATTERN = r'^# CONFIG_(\w+) is not set$' CONFIG_PATTERN = r'^CONFIG_(\w+)=(\S+|".*")$' @@ -32,35 +32,46 @@ class Kconfig: """Represents defconfig or .config specified using the Kconfig language.""" def __init__(self) -> None: - self._entries = [] # type: List[KconfigEntry] + self._entries = {} # type: Dict[str, str] - def entries(self) -> Set[KconfigEntry]: - return set(self._entries) + def __eq__(self, other) -> bool: + if not isinstance(other, self.__class__): + return False + return self._entries == other._entries - def add_entry(self, entry: KconfigEntry) -> None: - self._entries.append(entry) + def __repr__(self) -> str: + return ','.join(str(e) for e in self._as_entries()) + + + def _as_entries(self) -> Iterable[KconfigEntry]: + for name, value in self._entries.items(): + yield KconfigEntry(name, value) + + def add_entry(self, name: str, value: str) -> None: + self._entries[name] = value def is_subset_of(self, other: 'Kconfig') -> bool: - other_dict = {e.name: e.value for e in other.entries()} - for a in self.entries(): - b = other_dict.get(a.name) + for name, value in self._entries.items(): + b = other._entries.get(name) if b is None: - if a.value == 'n': + if value == 'n': continue return False - if a.value != b: + if value != b: return False return True + def set_diff(self, other: 'Kconfig') -> Set[KconfigEntry]: + return set(self._as_entries()) - set(other._as_entries()) + def merge_in_entries(self, other: 'Kconfig') -> None: - if other.is_subset_of(self): - return - self._entries = list(self.entries().union(other.entries())) + for name, value in other._entries.items(): + self._entries[name] = value def write_to_file(self, path: str) -> None: with open(path, 'a+') as f: - for entry in self.entries(): - f.write(str(entry) + '\n') + for e in self._as_entries(): + f.write(str(e) + '\n') def parse_file(path: str) -> Kconfig: with open(path, 'r') as f: @@ -78,14 +89,12 @@ def parse_from_string(blob: str) -> Kconfig: match = config_matcher.match(line) if match: - entry = KconfigEntry(match.group(1), match.group(2)) - kconfig.add_entry(entry) + kconfig.add_entry(match.group(1), match.group(2)) continue empty_match = is_not_set_matcher.match(line) if empty_match: - entry = KconfigEntry(empty_match.group(1), 'n') - kconfig.add_entry(entry) + kconfig.add_entry(empty_match.group(1), 'n') continue if line[0] == '#': diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py index 3539efaf99ba..6d994bb24999 100644 --- a/tools/testing/kunit/kunit_kernel.py +++ b/tools/testing/kunit/kunit_kernel.py @@ -53,8 +53,8 @@ class LinuxSourceTreeOperations: except subprocess.CalledProcessError as e: raise ConfigError(e.output.decode()) - def make_arch_qemuconfig(self, base_kunitconfig: kunit_config.Kconfig) -> None: - pass + def make_arch_qemuconfig(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig: + return base_kunitconfig def make_allyesconfig(self, build_dir: str, make_options) -> None: raise ConfigError('Only the "um" arch is supported for alltests') @@ -109,9 +109,10 @@ class LinuxSourceTreeOperationsQemu(LinuxSourceTreeOperations): self._kernel_command_line = qemu_arch_params.kernel_command_line + ' kunit_shutdown=reboot' self._extra_qemu_params = qemu_arch_params.extra_qemu_params - def make_arch_qemuconfig(self, base_kunitconfig: kunit_config.Kconfig) -> None: + def make_arch_qemuconfig(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig: kconfig = kunit_config.parse_from_string(self._kconfig) - base_kunitconfig.merge_in_entries(kconfig) + kconfig.merge_in_entries(base_kunitconfig) + return kconfig def start(self, params: List[str], build_dir: str) -> subprocess.Popen: kernel_path = os.path.join(build_dir, self._kernel_path) @@ -267,7 +268,7 @@ class LinuxSourceTree: validated_kconfig = kunit_config.parse_file(kconfig_path) if self._kconfig.is_subset_of(validated_kconfig): return True - invalid = self._kconfig.entries() - validated_kconfig.entries() + invalid = self._kconfig.set_diff(validated_kconfig) message = 'Not all Kconfig options selected in kunitconfig were in the generated .config.\n' \ 'This is probably due to unsatisfied dependencies.\n' \ 'Missing: ' + ', '.join([str(e) for e in invalid]) @@ -282,7 +283,7 @@ class LinuxSourceTree: if build_dir and not os.path.exists(build_dir): os.mkdir(build_dir) try: - self._ops.make_arch_qemuconfig(self._kconfig) + self._kconfig = self._ops.make_arch_qemuconfig(self._kconfig) self._kconfig.write_to_file(kconfig_path) self._ops.make_olddefconfig(build_dir, make_options) except ConfigError as e: @@ -303,7 +304,7 @@ class LinuxSourceTree: return True old_kconfig = kunit_config.parse_file(old_path) - return old_kconfig.entries() != self._kconfig.entries() + return old_kconfig != self._kconfig def build_reconfig(self, build_dir: str, make_options) -> bool: """Creates a new .config if it is not a subset of the .kunitconfig.""" @@ -313,7 +314,8 @@ class LinuxSourceTree: return self.build_config(build_dir, make_options) existing_kconfig = kunit_config.parse_file(kconfig_path) - self._ops.make_arch_qemuconfig(self._kconfig) + self._kconfig = self._ops.make_arch_qemuconfig(self._kconfig) + if self._kconfig.is_subset_of(existing_kconfig) and not self._kunitconfig_changed(build_dir): return True print('Regenerating .config ...') diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py index 25a2eb3bf114..3a8f638ff092 100755 --- a/tools/testing/kunit/kunit_tool_test.py +++ b/tools/testing/kunit/kunit_tool_test.py @@ -45,7 +45,7 @@ class KconfigTest(unittest.TestCase): self.assertTrue(kconfig0.is_subset_of(kconfig0)) kconfig1 = kunit_config.Kconfig() - kconfig1.add_entry(kunit_config.KconfigEntry('TEST', 'y')) + kconfig1.add_entry('TEST', 'y') self.assertTrue(kconfig1.is_subset_of(kconfig1)) self.assertTrue(kconfig0.is_subset_of(kconfig1)) self.assertFalse(kconfig1.is_subset_of(kconfig0)) @@ -56,40 +56,28 @@ class KconfigTest(unittest.TestCase): kconfig = kunit_config.parse_file(kconfig_path) expected_kconfig = kunit_config.Kconfig() - expected_kconfig.add_entry( - kunit_config.KconfigEntry('UML', 'y')) - expected_kconfig.add_entry( - kunit_config.KconfigEntry('MMU', 'y')) - expected_kconfig.add_entry( - kunit_config.KconfigEntry('TEST', 'y')) - expected_kconfig.add_entry( - kunit_config.KconfigEntry('EXAMPLE_TEST', 'y')) - expected_kconfig.add_entry( - kunit_config.KconfigEntry('MK8', 'n')) - - self.assertEqual(kconfig.entries(), expected_kconfig.entries()) + expected_kconfig.add_entry('UML', 'y') + expected_kconfig.add_entry('MMU', 'y') + expected_kconfig.add_entry('TEST', 'y') + expected_kconfig.add_entry('EXAMPLE_TEST', 'y') + expected_kconfig.add_entry('MK8', 'n') + + self.assertEqual(kconfig, expected_kconfig) def test_write_to_file(self): kconfig_path = os.path.join(test_tmpdir, '.config') expected_kconfig = kunit_config.Kconfig() - expected_kconfig.add_entry( - kunit_config.KconfigEntry('UML', 'y')) - expected_kconfig.add_entry( - kunit_config.KconfigEntry('MMU', 'y')) - expected_kconfig.add_entry( - kunit_config.KconfigEntry('TEST', 'y')) - expected_kconfig.add_entry( - kunit_config.KconfigEntry('EXAMPLE_TEST', 'y')) - expected_kconfig.add_entry( - kunit_config.KconfigEntry('MK8', 'n')) + expected_kconfig.add_entry('UML', 'y') + expected_kconfig.add_entry('MMU', 'y') + expected_kconfig.add_entry('TEST', 'y') + expected_kconfig.add_entry('EXAMPLE_TEST', 'y') + expected_kconfig.add_entry('MK8', 'n') expected_kconfig.write_to_file(kconfig_path) actual_kconfig = kunit_config.parse_file(kconfig_path) - - self.assertEqual(actual_kconfig.entries(), - expected_kconfig.entries()) + self.assertEqual(actual_kconfig, expected_kconfig) class KUnitParserTest(unittest.TestCase): @@ -381,8 +369,11 @@ class LinuxSourceTreeTest(unittest.TestCase): kunit_kernel.LinuxSourceTree('', kunitconfig_path=dir) def test_kconfig_add(self): + want_kconfig = kunit_config.Kconfig() + want_kconfig.add_entry('NOT_REAL', 'y') + tree = kunit_kernel.LinuxSourceTree('', kconfig_add=['CONFIG_NOT_REAL=y']) - self.assertIn(kunit_config.KconfigEntry('NOT_REAL', 'y'), tree._kconfig.entries()) + self.assertFalse(want_kconfig.set_diff(tree._kconfig)) def test_invalid_arch(self): with self.assertRaisesRegex(kunit_kernel.ConfigError, 'not a valid arch, options are.*x86_64'):