2022-06-24 09:00:20

by David Gow

[permalink] [raw]
Subject: [PATCH] kunit: tool: Enable virtio/PCI by default on UML

There are several tests which depend on PCI, and hence need a bunch of
extra options to run under UML. This makes it awkward to give
configuration instructions (whether in documentation, or as part of a
.kunitconfig file), as two separate, incompatible sets of config options
are required for UML and "most other architectures".

For non-UML architectures, it's possible to add default kconfig options
via the qemu_config python files, but there's no equivalent for UML. Add
a new tools/testing/kunit/configs/arch_uml.config file containing extra
kconfig options to use on UML.

Signed-off-by: David Gow <[email protected]>
---

NOTE: This has dependencies on the 'make --kunitconfig repeatable'
series:
https://lore.kernel.org/linux-kselftest/[email protected]/
which, in turn, depends on:
https://lore.kernel.org/linux-kselftest/[email protected]/
Please apply those first.

Changes since RFC:
https://lore.kernel.org/linux-kselftest/[email protected]/
- Rebase on top of the previous kconfig patches.
- Fix a missing make_arch_qemuconfig->make_arch_config rename (Thanks
Brendan)
- Fix the tests to use the base LinuxSourceTreeOperations class, which
has no default kconfig options (and so won't conflict with those set
in the tests). Only test_build_reconfig_existing_config actually
failed, but I updated a few more in case the defaults changed.


---
tools/testing/kunit/configs/arch_uml.config | 5 +++++
tools/testing/kunit/kunit_kernel.py | 14 ++++++++++----
tools/testing/kunit/kunit_tool_test.py | 12 ++++++++++++
3 files changed, 27 insertions(+), 4 deletions(-)
create mode 100644 tools/testing/kunit/configs/arch_uml.config

diff --git a/tools/testing/kunit/configs/arch_uml.config b/tools/testing/kunit/configs/arch_uml.config
new file mode 100644
index 000000000000..e824ce43b05a
--- /dev/null
+++ b/tools/testing/kunit/configs/arch_uml.config
@@ -0,0 +1,5 @@
+# Config options which are added to UML builds by default
+
+# Enable virtio/pci, as a lot of tests require it.
+CONFIG_VIRTIO_UML=y
+CONFIG_UML_PCI_OVER_VIRTIO=y
diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
index fc415ff7530e..127598fb994b 100644
--- a/tools/testing/kunit/kunit_kernel.py
+++ b/tools/testing/kunit/kunit_kernel.py
@@ -26,6 +26,7 @@ KUNITCONFIG_PATH = '.kunitconfig'
OLD_KUNITCONFIG_PATH = 'last_used_kunitconfig'
DEFAULT_KUNITCONFIG_PATH = 'tools/testing/kunit/configs/default.config'
BROKEN_ALLCONFIG_PATH = 'tools/testing/kunit/configs/broken_on_uml.config'
+UML_KCONFIG_PATH = 'tools/testing/kunit/configs/arch_uml.config'
OUTFILE_PATH = 'test.log'
ABS_TOOL_PATH = os.path.abspath(os.path.dirname(__file__))
QEMU_CONFIGS_DIR = os.path.join(ABS_TOOL_PATH, 'qemu_configs')
@@ -53,7 +54,7 @@ class LinuxSourceTreeOperations:
except subprocess.CalledProcessError as e:
raise ConfigError(e.output.decode())

- def make_arch_qemuconfig(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig:
+ def make_arch_config(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig:
return base_kunitconfig

def make_allyesconfig(self, build_dir: str, make_options) -> None:
@@ -109,7 +110,7 @@ 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) -> kunit_config.Kconfig:
+ def make_arch_config(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig:
kconfig = kunit_config.parse_from_string(self._kconfig)
kconfig.merge_in_entries(base_kunitconfig)
return kconfig
@@ -138,6 +139,11 @@ class LinuxSourceTreeOperationsUml(LinuxSourceTreeOperations):
def __init__(self, cross_compile=None):
super().__init__(linux_arch='um', cross_compile=cross_compile)

+ def make_arch_config(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig:
+ kconfig = kunit_config.parse_file(UML_KCONFIG_PATH)
+ kconfig.merge_in_entries(base_kunitconfig)
+ return kconfig
+
def make_allyesconfig(self, build_dir: str, make_options) -> None:
kunit_parser.print_with_timestamp(
'Enabling all CONFIGs for UML...')
@@ -297,7 +303,7 @@ class LinuxSourceTree:
if build_dir and not os.path.exists(build_dir):
os.mkdir(build_dir)
try:
- self._kconfig = self._ops.make_arch_qemuconfig(self._kconfig)
+ self._kconfig = self._ops.make_arch_config(self._kconfig)
self._kconfig.write_to_file(kconfig_path)
self._ops.make_olddefconfig(build_dir, make_options)
except ConfigError as e:
@@ -328,7 +334,7 @@ class LinuxSourceTree:
return self.build_config(build_dir, make_options)

existing_kconfig = kunit_config.parse_file(kconfig_path)
- self._kconfig = self._ops.make_arch_qemuconfig(self._kconfig)
+ self._kconfig = self._ops.make_arch_config(self._kconfig)

if self._kconfig.is_subset_of(existing_kconfig) and not self._kunitconfig_changed(build_dir):
return True
diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
index e21ae1331350..08cb2dc8ef7d 100755
--- a/tools/testing/kunit/kunit_tool_test.py
+++ b/tools/testing/kunit/kunit_tool_test.py
@@ -430,6 +430,10 @@ class LinuxSourceTreeTest(unittest.TestCase):
f.write('CONFIG_KUNIT=y')

tree = kunit_kernel.LinuxSourceTree(build_dir)
+ # Stub out the source tree operations, so we don't have
+ # the defaults for any given architecture get in the
+ # way.
+ tree._ops = kunit_kernel.LinuxSourceTreeOperations(None, None)
mock_build_config = mock.patch.object(tree, 'build_config').start()

# Should generate the .config
@@ -447,6 +451,10 @@ class LinuxSourceTreeTest(unittest.TestCase):
f.write('CONFIG_KUNIT=y\nCONFIG_KUNIT_TEST=y')

tree = kunit_kernel.LinuxSourceTree(build_dir)
+ # Stub out the source tree operations, so we don't have
+ # the defaults for any given architecture get in the
+ # way.
+ tree._ops = kunit_kernel.LinuxSourceTreeOperations(None, None)
mock_build_config = mock.patch.object(tree, 'build_config').start()

self.assertTrue(tree.build_reconfig(build_dir, make_options=[]))
@@ -463,6 +471,10 @@ class LinuxSourceTreeTest(unittest.TestCase):
f.write('CONFIG_KUNIT=y\nCONFIG_KUNIT_TEST=y')

tree = kunit_kernel.LinuxSourceTree(build_dir)
+ # Stub out the source tree operations, so we don't have
+ # the defaults for any given architecture get in the
+ # way.
+ tree._ops = kunit_kernel.LinuxSourceTreeOperations(None, None)
mock_build_config = mock.patch.object(tree, 'build_config').start()

# ... so we should trigger a call to build_config()
--
2.37.0.rc0.161.g10f37bed90-goog


2022-06-27 07:34:15

by José Expósito

[permalink] [raw]
Subject: Re: [PATCH] kunit: tool: Enable virtio/PCI by default on UML

On Fri, Jun 24, 2022 at 04:43:59PM +0800, David Gow wrote:
> There are several tests which depend on PCI, and hence need a bunch of
> extra options to run under UML. This makes it awkward to give
> configuration instructions (whether in documentation, or as part of a
> .kunitconfig file), as two separate, incompatible sets of config options
> are required for UML and "most other architectures".
>
> For non-UML architectures, it's possible to add default kconfig options
> via the qemu_config python files, but there's no equivalent for UML. Add
> a new tools/testing/kunit/configs/arch_uml.config file containing extra
> kconfig options to use on UML.
>
> Signed-off-by: David Gow <[email protected]>

Tested-by: Jos? Exp?sito <[email protected]>

After applying this patch and its dependencies, I can confirm that it is
possible to run the DRM tests with the following command:

$ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/tests

Added a TODO for myself to remove the extra flag from the DRM docs once
this patch is merged.

Thanks a lot for making it easier!

Jose

> ---
>
> NOTE: This has dependencies on the 'make --kunitconfig repeatable'
> series:
> https://lore.kernel.org/linux-kselftest/[email protected]/
> which, in turn, depends on:
> https://lore.kernel.org/linux-kselftest/[email protected]/
> Please apply those first.
>
> Changes since RFC:
> https://lore.kernel.org/linux-kselftest/[email protected]/
> - Rebase on top of the previous kconfig patches.
> - Fix a missing make_arch_qemuconfig->make_arch_config rename (Thanks
> Brendan)
> - Fix the tests to use the base LinuxSourceTreeOperations class, which
> has no default kconfig options (and so won't conflict with those set
> in the tests). Only test_build_reconfig_existing_config actually
> failed, but I updated a few more in case the defaults changed.
>
>
> ---
> tools/testing/kunit/configs/arch_uml.config | 5 +++++
> tools/testing/kunit/kunit_kernel.py | 14 ++++++++++----
> tools/testing/kunit/kunit_tool_test.py | 12 ++++++++++++
> 3 files changed, 27 insertions(+), 4 deletions(-)
> create mode 100644 tools/testing/kunit/configs/arch_uml.config
>
> diff --git a/tools/testing/kunit/configs/arch_uml.config b/tools/testing/kunit/configs/arch_uml.config
> new file mode 100644
> index 000000000000..e824ce43b05a
> --- /dev/null
> +++ b/tools/testing/kunit/configs/arch_uml.config
> @@ -0,0 +1,5 @@
> +# Config options which are added to UML builds by default
> +
> +# Enable virtio/pci, as a lot of tests require it.
> +CONFIG_VIRTIO_UML=y
> +CONFIG_UML_PCI_OVER_VIRTIO=y
> diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
> index fc415ff7530e..127598fb994b 100644
> --- a/tools/testing/kunit/kunit_kernel.py
> +++ b/tools/testing/kunit/kunit_kernel.py
> @@ -26,6 +26,7 @@ KUNITCONFIG_PATH = '.kunitconfig'
> OLD_KUNITCONFIG_PATH = 'last_used_kunitconfig'
> DEFAULT_KUNITCONFIG_PATH = 'tools/testing/kunit/configs/default.config'
> BROKEN_ALLCONFIG_PATH = 'tools/testing/kunit/configs/broken_on_uml.config'
> +UML_KCONFIG_PATH = 'tools/testing/kunit/configs/arch_uml.config'
> OUTFILE_PATH = 'test.log'
> ABS_TOOL_PATH = os.path.abspath(os.path.dirname(__file__))
> QEMU_CONFIGS_DIR = os.path.join(ABS_TOOL_PATH, 'qemu_configs')
> @@ -53,7 +54,7 @@ class LinuxSourceTreeOperations:
> except subprocess.CalledProcessError as e:
> raise ConfigError(e.output.decode())
>
> - def make_arch_qemuconfig(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig:
> + def make_arch_config(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig:
> return base_kunitconfig
>
> def make_allyesconfig(self, build_dir: str, make_options) -> None:
> @@ -109,7 +110,7 @@ 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) -> kunit_config.Kconfig:
> + def make_arch_config(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig:
> kconfig = kunit_config.parse_from_string(self._kconfig)
> kconfig.merge_in_entries(base_kunitconfig)
> return kconfig
> @@ -138,6 +139,11 @@ class LinuxSourceTreeOperationsUml(LinuxSourceTreeOperations):
> def __init__(self, cross_compile=None):
> super().__init__(linux_arch='um', cross_compile=cross_compile)
>
> + def make_arch_config(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig:
> + kconfig = kunit_config.parse_file(UML_KCONFIG_PATH)
> + kconfig.merge_in_entries(base_kunitconfig)
> + return kconfig
> +
> def make_allyesconfig(self, build_dir: str, make_options) -> None:
> kunit_parser.print_with_timestamp(
> 'Enabling all CONFIGs for UML...')
> @@ -297,7 +303,7 @@ class LinuxSourceTree:
> if build_dir and not os.path.exists(build_dir):
> os.mkdir(build_dir)
> try:
> - self._kconfig = self._ops.make_arch_qemuconfig(self._kconfig)
> + self._kconfig = self._ops.make_arch_config(self._kconfig)
> self._kconfig.write_to_file(kconfig_path)
> self._ops.make_olddefconfig(build_dir, make_options)
> except ConfigError as e:
> @@ -328,7 +334,7 @@ class LinuxSourceTree:
> return self.build_config(build_dir, make_options)
>
> existing_kconfig = kunit_config.parse_file(kconfig_path)
> - self._kconfig = self._ops.make_arch_qemuconfig(self._kconfig)
> + self._kconfig = self._ops.make_arch_config(self._kconfig)
>
> if self._kconfig.is_subset_of(existing_kconfig) and not self._kunitconfig_changed(build_dir):
> return True
> diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
> index e21ae1331350..08cb2dc8ef7d 100755
> --- a/tools/testing/kunit/kunit_tool_test.py
> +++ b/tools/testing/kunit/kunit_tool_test.py
> @@ -430,6 +430,10 @@ class LinuxSourceTreeTest(unittest.TestCase):
> f.write('CONFIG_KUNIT=y')
>
> tree = kunit_kernel.LinuxSourceTree(build_dir)
> + # Stub out the source tree operations, so we don't have
> + # the defaults for any given architecture get in the
> + # way.
> + tree._ops = kunit_kernel.LinuxSourceTreeOperations(None, None)
> mock_build_config = mock.patch.object(tree, 'build_config').start()
>
> # Should generate the .config
> @@ -447,6 +451,10 @@ class LinuxSourceTreeTest(unittest.TestCase):
> f.write('CONFIG_KUNIT=y\nCONFIG_KUNIT_TEST=y')
>
> tree = kunit_kernel.LinuxSourceTree(build_dir)
> + # Stub out the source tree operations, so we don't have
> + # the defaults for any given architecture get in the
> + # way.
> + tree._ops = kunit_kernel.LinuxSourceTreeOperations(None, None)
> mock_build_config = mock.patch.object(tree, 'build_config').start()
>
> self.assertTrue(tree.build_reconfig(build_dir, make_options=[]))
> @@ -463,6 +471,10 @@ class LinuxSourceTreeTest(unittest.TestCase):
> f.write('CONFIG_KUNIT=y\nCONFIG_KUNIT_TEST=y')
>
> tree = kunit_kernel.LinuxSourceTree(build_dir)
> + # Stub out the source tree operations, so we don't have
> + # the defaults for any given architecture get in the
> + # way.
> + tree._ops = kunit_kernel.LinuxSourceTreeOperations(None, None)
> mock_build_config = mock.patch.object(tree, 'build_config').start()
>
> # ... so we should trigger a call to build_config()
> --
> 2.37.0.rc0.161.g10f37bed90-goog
>

2022-06-27 23:21:11

by Daniel Latypov

[permalink] [raw]
Subject: Re: [PATCH] kunit: tool: Enable virtio/PCI by default on UML

On Fri, Jun 24, 2022 at 1:44 AM David Gow <[email protected]> wrote:
>
> There are several tests which depend on PCI, and hence need a bunch of
> extra options to run under UML. This makes it awkward to give
> configuration instructions (whether in documentation, or as part of a
> .kunitconfig file), as two separate, incompatible sets of config options
> are required for UML and "most other architectures".
>
> For non-UML architectures, it's possible to add default kconfig options
> via the qemu_config python files, but there's no equivalent for UML. Add
> a new tools/testing/kunit/configs/arch_uml.config file containing extra
> kconfig options to use on UML.
>
> Signed-off-by: David Gow <[email protected]>
> ---
>
> NOTE: This has dependencies on the 'make --kunitconfig repeatable'
> series:
> https://lore.kernel.org/linux-kselftest/[email protected]/
> which, in turn, depends on:
> https://lore.kernel.org/linux-kselftest/[email protected]/
> Please apply those first.
>
> Changes since RFC:
> https://lore.kernel.org/linux-kselftest/[email protected]/
> - Rebase on top of the previous kconfig patches.
> - Fix a missing make_arch_qemuconfig->make_arch_config rename (Thanks
> Brendan)
> - Fix the tests to use the base LinuxSourceTreeOperations class, which
> has no default kconfig options (and so won't conflict with those set
> in the tests). Only test_build_reconfig_existing_config actually
> failed, but I updated a few more in case the defaults changed.
>
>
> ---
> tools/testing/kunit/configs/arch_uml.config | 5 +++++
> tools/testing/kunit/kunit_kernel.py | 14 ++++++++++----
> tools/testing/kunit/kunit_tool_test.py | 12 ++++++++++++
> 3 files changed, 27 insertions(+), 4 deletions(-)
> create mode 100644 tools/testing/kunit/configs/arch_uml.config
>
> diff --git a/tools/testing/kunit/configs/arch_uml.config b/tools/testing/kunit/configs/arch_uml.config
> new file mode 100644
> index 000000000000..e824ce43b05a
> --- /dev/null
> +++ b/tools/testing/kunit/configs/arch_uml.config
> @@ -0,0 +1,5 @@
> +# Config options which are added to UML builds by default
> +
> +# Enable virtio/pci, as a lot of tests require it.
> +CONFIG_VIRTIO_UML=y
> +CONFIG_UML_PCI_OVER_VIRTIO=y
> diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
> index fc415ff7530e..127598fb994b 100644
> --- a/tools/testing/kunit/kunit_kernel.py
> +++ b/tools/testing/kunit/kunit_kernel.py
> @@ -26,6 +26,7 @@ KUNITCONFIG_PATH = '.kunitconfig'
> OLD_KUNITCONFIG_PATH = 'last_used_kunitconfig'
> DEFAULT_KUNITCONFIG_PATH = 'tools/testing/kunit/configs/default.config'
> BROKEN_ALLCONFIG_PATH = 'tools/testing/kunit/configs/broken_on_uml.config'
> +UML_KCONFIG_PATH = 'tools/testing/kunit/configs/arch_uml.config'
> OUTFILE_PATH = 'test.log'
> ABS_TOOL_PATH = os.path.abspath(os.path.dirname(__file__))
> QEMU_CONFIGS_DIR = os.path.join(ABS_TOOL_PATH, 'qemu_configs')
> @@ -53,7 +54,7 @@ class LinuxSourceTreeOperations:
> except subprocess.CalledProcessError as e:
> raise ConfigError(e.output.decode())
>
> - def make_arch_qemuconfig(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig:
> + def make_arch_config(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig:
> return base_kunitconfig
>
> def make_allyesconfig(self, build_dir: str, make_options) -> None:
> @@ -109,7 +110,7 @@ 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) -> kunit_config.Kconfig:
> + def make_arch_config(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig:
> kconfig = kunit_config.parse_from_string(self._kconfig)
> kconfig.merge_in_entries(base_kunitconfig)
> return kconfig
> @@ -138,6 +139,11 @@ class LinuxSourceTreeOperationsUml(LinuxSourceTreeOperations):
> def __init__(self, cross_compile=None):
> super().__init__(linux_arch='um', cross_compile=cross_compile)
>
> + def make_arch_config(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig:
> + kconfig = kunit_config.parse_file(UML_KCONFIG_PATH)
> + kconfig.merge_in_entries(base_kunitconfig)
> + return kconfig
> +
> def make_allyesconfig(self, build_dir: str, make_options) -> None:
> kunit_parser.print_with_timestamp(
> 'Enabling all CONFIGs for UML...')
> @@ -297,7 +303,7 @@ class LinuxSourceTree:
> if build_dir and not os.path.exists(build_dir):
> os.mkdir(build_dir)
> try:
> - self._kconfig = self._ops.make_arch_qemuconfig(self._kconfig)
> + self._kconfig = self._ops.make_arch_config(self._kconfig)
> self._kconfig.write_to_file(kconfig_path)
> self._ops.make_olddefconfig(build_dir, make_options)
> except ConfigError as e:
> @@ -328,7 +334,7 @@ class LinuxSourceTree:
> return self.build_config(build_dir, make_options)
>
> existing_kconfig = kunit_config.parse_file(kconfig_path)
> - self._kconfig = self._ops.make_arch_qemuconfig(self._kconfig)
> + self._kconfig = self._ops.make_arch_config(self._kconfig)
>
> if self._kconfig.is_subset_of(existing_kconfig) and not self._kunitconfig_changed(build_dir):
> return True
> diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
> index e21ae1331350..08cb2dc8ef7d 100755
> --- a/tools/testing/kunit/kunit_tool_test.py
> +++ b/tools/testing/kunit/kunit_tool_test.py
> @@ -430,6 +430,10 @@ class LinuxSourceTreeTest(unittest.TestCase):
> f.write('CONFIG_KUNIT=y')
>
> tree = kunit_kernel.LinuxSourceTree(build_dir)
> + # Stub out the source tree operations, so we don't have
> + # the defaults for any given architecture get in the
> + # way.
> + tree._ops = kunit_kernel.LinuxSourceTreeOperations(None, None)

This runs and typechecks under mypy, but not under pytype.
The problem is that the first argument is type str, not Optional[str].

I think a fix would be to just use LinuxSourceTreeOperationsUml() instead here.

Since you recently switched machines, you might want to run:
$ pip install pytype

And then
$ ./tools/testing/kunit/run_checks.py
would run pytype and show you the complaints here.



> mock_build_config = mock.patch.object(tree, 'build_config').start()
>
> # Should generate the .config
> @@ -447,6 +451,10 @@ class LinuxSourceTreeTest(unittest.TestCase):
> f.write('CONFIG_KUNIT=y\nCONFIG_KUNIT_TEST=y')
>
> tree = kunit_kernel.LinuxSourceTree(build_dir)
> + # Stub out the source tree operations, so we don't have
> + # the defaults for any given architecture get in the
> + # way.
> + tree._ops = kunit_kernel.LinuxSourceTreeOperations(None, None)
> mock_build_config = mock.patch.object(tree, 'build_config').start()
>
> self.assertTrue(tree.build_reconfig(build_dir, make_options=[]))
> @@ -463,6 +471,10 @@ class LinuxSourceTreeTest(unittest.TestCase):
> f.write('CONFIG_KUNIT=y\nCONFIG_KUNIT_TEST=y')
>
> tree = kunit_kernel.LinuxSourceTree(build_dir)
> + # Stub out the source tree operations, so we don't have
> + # the defaults for any given architecture get in the
> + # way.
> + tree._ops = kunit_kernel.LinuxSourceTreeOperations(None, None)
> mock_build_config = mock.patch.object(tree, 'build_config').start()
>
> # ... so we should trigger a call to build_config()
> --
> 2.37.0.rc0.161.g10f37bed90-goog
>

2022-06-28 00:14:22

by Daniel Latypov

[permalink] [raw]
Subject: Re: [PATCH] kunit: tool: Enable virtio/PCI by default on UML

On Mon, Jun 27, 2022 at 3:57 PM Daniel Latypov <[email protected]> wrote:
>
> On Fri, Jun 24, 2022 at 1:44 AM David Gow <[email protected]> wrote:
> >
> > There are several tests which depend on PCI, and hence need a bunch of
> > extra options to run under UML. This makes it awkward to give
> > configuration instructions (whether in documentation, or as part of a
> > .kunitconfig file), as two separate, incompatible sets of config options
> > are required for UML and "most other architectures".
> >
> > For non-UML architectures, it's possible to add default kconfig options
> > via the qemu_config python files, but there's no equivalent for UML. Add
> > a new tools/testing/kunit/configs/arch_uml.config file containing extra
> > kconfig options to use on UML.
> >
> > Signed-off-by: David Gow <[email protected]>

Reviewed-by: Daniel Latypov <[email protected]>

LGTM, modulo the pytype error mentioned before.

> > tree = kunit_kernel.LinuxSourceTree(build_dir)
> > + # Stub out the source tree operations, so we don't have
> > + # the defaults for any given architecture get in the
> > + # way.
> > + tree._ops = kunit_kernel.LinuxSourceTreeOperations(None, None)
>
> This runs and typechecks under mypy, but not under pytype.
> The problem is that the first argument is type str, not Optional[str].
>
> I think a fix would be to just use LinuxSourceTreeOperationsUml() instead here.
>
> Since you recently switched machines, you might want to run:
> $ pip install pytype
>
> And then
> $ ./tools/testing/kunit/run_checks.py
> would run pytype and show you the complaints here.

Oh, I see what you're doing here, we want to avoid the new step where
UML now adds to the .kunitconfig file.

Something like this could work
- tree._ops = kunit_kernel.LinuxSourceTreeOperations(None, None)
+ tree._ops = kunit_kernel.LinuxSourceTreeOperations('none', None)
or we could put 'fake', etc.

If we're not happy with using this class directly (since it's meant to
subclassed), an alternative, more targeted approach could be:
mock.patch.object(tree._ops, 'make_arch_config', lambda x: x).start()
But I don't like this and would prefer the above.

Daniel