Message ID | 20211022004936.2049804-1-dlatypov@google.com (mailing list archive) |
---|---|
State | Accepted |
Commit | 52a5d80a2225e2d0b2a8f4656b76aead2a443b2a |
Delegated to: | Brendan Higgins |
Headers | show |
Series | kunit: tool: fix typecheck errors about loading qemu configs | expand |
On Fri, Oct 22, 2021 at 8:49 AM 'Daniel Latypov' via KUnit Development <kunit-dev@googlegroups.com> wrote: > > Currently, we have these errors: > $ mypy ./tools/testing/kunit/*.py > tools/testing/kunit/kunit_kernel.py:213: error: Item "_Loader" of "Optional[_Loader]" has no attribute "exec_module" > tools/testing/kunit/kunit_kernel.py:213: error: Item "None" of "Optional[_Loader]" has no attribute "exec_module" > tools/testing/kunit/kunit_kernel.py:214: error: Module has no attribute "QEMU_ARCH" > tools/testing/kunit/kunit_kernel.py:215: error: Module has no attribute "QEMU_ARCH" > > exec_module > =========== > > pytype currently reports no errors, but that's because there's a comment > disabling it on 213. > > This is due to https://github.com/python/typeshed/pull/2626. > The fix is to assert the loaded module implements the ABC > (abstract base class) we want which has exec_module support. > > QEMU_ARCH > ========= > > pytype is fine with this, but mypy is not: > https://github.com/python/mypy/issues/5059 > > Add a check that the loaded module does indeed have QEMU_ARCH. > Note: this is not enough to appease mypy, so we also add a comment to > squash the warning. > > Signed-off-by: Daniel Latypov <dlatypov@google.com> > --- Thanks -- this has been annoying me quite a bit, so it's good to have it fixed. It's a bit of a shame mypy needs the additional comment, but what can you do... Reviewed-by: David Gow <davidgow@google.com> Cheers, -- David > tools/testing/kunit/kunit_kernel.py | 15 +++++++++------ > 1 file changed, 9 insertions(+), 6 deletions(-) > > diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py > index faa6320e900e..c68b17905481 100644 > --- a/tools/testing/kunit/kunit_kernel.py > +++ b/tools/testing/kunit/kunit_kernel.py > @@ -207,12 +207,15 @@ def get_source_tree_ops_from_qemu_config(config_path: str, > module_path = '.' + os.path.join(os.path.basename(QEMU_CONFIGS_DIR), os.path.basename(config_path)) > spec = importlib.util.spec_from_file_location(module_path, config_path) > config = importlib.util.module_from_spec(spec) > - # TODO(brendanhiggins@google.com): I looked this up and apparently other > - # Python projects have noted that pytype complains that "No attribute > - # 'exec_module' on _importlib_modulespec._Loader". Disabling for now. > - spec.loader.exec_module(config) # pytype: disable=attribute-error > - return config.QEMU_ARCH.linux_arch, LinuxSourceTreeOperationsQemu( > - config.QEMU_ARCH, cross_compile=cross_compile) > + # See https://github.com/python/typeshed/pull/2626 for context. > + assert isinstance(spec.loader, importlib.abc.Loader) > + spec.loader.exec_module(config) > + > + if not hasattr(config, 'QEMU_ARCH'): > + raise ValueError('qemu_config module missing "QEMU_ARCH": ' + config_path) > + params: qemu_config.QemuArchParams = config.QEMU_ARCH # type: ignore > + return params.linux_arch, LinuxSourceTreeOperationsQemu( > + params, cross_compile=cross_compile) > > class LinuxSourceTree(object): > """Represents a Linux kernel source tree with KUnit tests.""" > > base-commit: 17ac23eb43f0cbefc8bfce44ad51a9f065895f9f > -- > 2.33.0.1079.g6e70778dc9-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@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20211022004936.2049804-1-dlatypov%40google.com.
On Thu, Oct 21, 2021 at 5:49 PM 'Daniel Latypov' via KUnit Development <kunit-dev@googlegroups.com> wrote: > > Currently, we have these errors: > $ mypy ./tools/testing/kunit/*.py > tools/testing/kunit/kunit_kernel.py:213: error: Item "_Loader" of "Optional[_Loader]" has no attribute "exec_module" > tools/testing/kunit/kunit_kernel.py:213: error: Item "None" of "Optional[_Loader]" has no attribute "exec_module" > tools/testing/kunit/kunit_kernel.py:214: error: Module has no attribute "QEMU_ARCH" > tools/testing/kunit/kunit_kernel.py:215: error: Module has no attribute "QEMU_ARCH" > > exec_module > =========== > > pytype currently reports no errors, but that's because there's a comment > disabling it on 213. > > This is due to https://github.com/python/typeshed/pull/2626. > The fix is to assert the loaded module implements the ABC > (abstract base class) we want which has exec_module support. > > QEMU_ARCH > ========= > > pytype is fine with this, but mypy is not: > https://github.com/python/mypy/issues/5059 > > Add a check that the loaded module does indeed have QEMU_ARCH. > Note: this is not enough to appease mypy, so we also add a comment to > squash the warning. > > Signed-off-by: Daniel Latypov <dlatypov@google.com> Thanks! I could not figure out how to make this work for both type checkers on my own. Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py index faa6320e900e..c68b17905481 100644 --- a/tools/testing/kunit/kunit_kernel.py +++ b/tools/testing/kunit/kunit_kernel.py @@ -207,12 +207,15 @@ def get_source_tree_ops_from_qemu_config(config_path: str, module_path = '.' + os.path.join(os.path.basename(QEMU_CONFIGS_DIR), os.path.basename(config_path)) spec = importlib.util.spec_from_file_location(module_path, config_path) config = importlib.util.module_from_spec(spec) - # TODO(brendanhiggins@google.com): I looked this up and apparently other - # Python projects have noted that pytype complains that "No attribute - # 'exec_module' on _importlib_modulespec._Loader". Disabling for now. - spec.loader.exec_module(config) # pytype: disable=attribute-error - return config.QEMU_ARCH.linux_arch, LinuxSourceTreeOperationsQemu( - config.QEMU_ARCH, cross_compile=cross_compile) + # See https://github.com/python/typeshed/pull/2626 for context. + assert isinstance(spec.loader, importlib.abc.Loader) + spec.loader.exec_module(config) + + if not hasattr(config, 'QEMU_ARCH'): + raise ValueError('qemu_config module missing "QEMU_ARCH": ' + config_path) + params: qemu_config.QemuArchParams = config.QEMU_ARCH # type: ignore + return params.linux_arch, LinuxSourceTreeOperationsQemu( + params, cross_compile=cross_compile) class LinuxSourceTree(object): """Represents a Linux kernel source tree with KUnit tests."""
Currently, we have these errors: $ mypy ./tools/testing/kunit/*.py tools/testing/kunit/kunit_kernel.py:213: error: Item "_Loader" of "Optional[_Loader]" has no attribute "exec_module" tools/testing/kunit/kunit_kernel.py:213: error: Item "None" of "Optional[_Loader]" has no attribute "exec_module" tools/testing/kunit/kunit_kernel.py:214: error: Module has no attribute "QEMU_ARCH" tools/testing/kunit/kunit_kernel.py:215: error: Module has no attribute "QEMU_ARCH" exec_module =========== pytype currently reports no errors, but that's because there's a comment disabling it on 213. This is due to https://github.com/python/typeshed/pull/2626. The fix is to assert the loaded module implements the ABC (abstract base class) we want which has exec_module support. QEMU_ARCH ========= pytype is fine with this, but mypy is not: https://github.com/python/mypy/issues/5059 Add a check that the loaded module does indeed have QEMU_ARCH. Note: this is not enough to appease mypy, so we also add a comment to squash the warning. Signed-off-by: Daniel Latypov <dlatypov@google.com> --- tools/testing/kunit/kunit_kernel.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) base-commit: 17ac23eb43f0cbefc8bfce44ad51a9f065895f9f