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

[Daniel Latypov]

On Mon, Jun 27, 2022 at 3:57 PM Daniel Latypov <dlatypov@xxxxxxxxxx> wrote:
>
> On Fri, Jun 24, 2022 at 1:44 AM David Gow <davidgow@xxxxxxxxxx> 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 <davidgow@xxxxxxxxxx>

Reviewed-by: Daniel Latypov <dlatypov@xxxxxxxxxx>

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