Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220624084400.1454579-1-davidgow@google.com> <CAGS_qxqrcyEH+C=63R8uT+K2opkXm6Da1wa4pyNupLUsy1iAKw@mail.gmail.com>
In-Reply-To: <CAGS_qxqrcyEH+C=63R8uT+K2opkXm6Da1wa4pyNupLUsy1iAKw@mail.gmail.com>
From:   Daniel Latypov <dlatypov@google.com>
Date:   Mon, 27 Jun 2022 16:33:54 -0700
Message-ID: <CAGS_qxra2cCoJvduj4Burtm+_wP2dg_hBFqwVZuhAkb38S4iyg@mail.gmail.com>
Subject: Re: [PATCH] kunit: tool: Enable virtio/PCI by default on UML
To:     David Gow <davidgow@google.com>
Cc:     Brendan Higgins <brendanhiggins@google.com>,
        Shuah Khan <skhan@linuxfoundation.org>,
        =?UTF-8?B?Sm9zw6kgRXhww7NzaXRv?= <jose.exposito89@gmail.com>,
        kunit-dev@googlegroups.com, linux-kselftest@vger.kernel.org,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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

Reviewed-by: Daniel Latypov <dlatypov@google.com>

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