From 443970004df71e1686c5b2e288340b9675100b3f Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Fri, 4 Sep 2026 16:33:53 +0000 Subject: [PATCH] unittests: Replace use of `from X import *` with explicit imports `*` imports that do not also make use of `__all__` are dangerous, they allow things to leak in that weren't inteded. An example of this is that we use `unittest` from the `*` import in several places. This not only makes the code cleaner, but helps analysis tools like mypy and flake8 to generate better warnings. --- unittests/allplatformstests.py | 20 +++++++++++--------- unittests/darwintests.py | 5 ++++- unittests/datatests.py | 3 ++- unittests/failuretests.py | 2 +- unittests/internaltests.py | 3 +-- unittests/linuxcrosstests.py | 4 +++- unittests/linuxliketests.py | 10 +++++++++- unittests/machinefiletests.py | 8 +++++--- unittests/optiontests.py | 7 +++++-- unittests/platformagnostictests.py | 6 +++--- unittests/pythontests.py | 2 +- unittests/subprojectscommandtests.py | 2 +- unittests/windowstests.py | 6 ++++-- 13 files changed, 50 insertions(+), 28 deletions(-) diff --git a/unittests/allplatformstests.py b/unittests/allplatformstests.py index ba69aafca..acc78672e 100644 --- a/unittests/allplatformstests.py +++ b/unittests/allplatformstests.py @@ -68,7 +68,10 @@ from run_tests import ( ) from .baseplatformtests import BasePlatformTests -from .helpers import * +from .helpers import ( + skip_if_not_language, skipIfNoExecutable, skip_if_no_cmake, + skipIfNoPkgconfig, IS_CI, chdir, get_rpath, skip_if_not_base_option, +) if T.TYPE_CHECKING: from mesonbuild.compilers.compilers import Language @@ -266,7 +269,6 @@ class AllPlatformTests(BasePlatformTests): with open(os.path.join(self.builddir, header), encoding='utf-8') as f: meson_header = f.read() - cmake_header_path = os.path.join(cmake_builddir, header) with open(os.path.join(cmake_builddir, header), encoding='utf-8') as f: cmake_header = f.read() @@ -5545,7 +5547,7 @@ class AllPlatformTests(BasePlatformTests): @skipIfNoExecutable('rustdoc') def test_rustdoc(self) -> None: if self.backend is not Backend.ninja: - raise unittest.SkipTest('Rust is only supported with ninja currently') + raise SkipTest('Rust is only supported with ninja currently') try: with tempfile.TemporaryDirectory() as tmpdir: testdir = os.path.join(tmpdir, 'a') @@ -5563,7 +5565,7 @@ class AllPlatformTests(BasePlatformTests): @skipIfNoExecutable('clippy-driver') def test_rust_clippy(self) -> None: if self.backend is not Backend.ninja: - raise unittest.SkipTest('Rust is only supported with ninja currently') + raise SkipTest('Rust is only supported with ninja currently') # When clippy is used, we should get an exception since a variable named # "foo" is used, but is on our denylist testdir = os.path.join(self.rust_test_dir, '1 basic') @@ -5581,7 +5583,7 @@ class AllPlatformTests(BasePlatformTests): @skipIfNoExecutable('clippy-driver') def test_rust_clippy_as_rustc(self) -> None: if self.backend is not Backend.ninja: - raise unittest.SkipTest('Rust is only supported with ninja currently') + raise SkipTest('Rust is only supported with ninja currently') # When clippy is used, we should get an exception since a variable named # "foo" is used, but is on our denylist testdir = os.path.join(self.rust_test_dir, '1 basic') @@ -5594,14 +5596,14 @@ class AllPlatformTests(BasePlatformTests): @skip_if_not_language('rust') def test_rust_test_warnings(self) -> None: if self.backend is not Backend.ninja: - raise unittest.SkipTest('Rust is only supported with ninja currently') + raise SkipTest('Rust is only supported with ninja currently') testdir = os.path.join(self.rust_test_dir, '9 unit tests') self.init(testdir, extra_args=['--fatal-meson-warnings']) @skip_if_not_language('rust') def test_rust_rlib_linkage(self) -> None: if self.backend is not Backend.ninja: - raise unittest.SkipTest('Rust is only supported with ninja currently') + raise SkipTest('Rust is only supported with ninja currently') template = textwrap.dedent('''\ use std::process::exit; @@ -5632,7 +5634,7 @@ class AllPlatformTests(BasePlatformTests): @skip_if_not_language('rust') def test_bindgen_drops_invalid(self) -> None: if self.backend is not Backend.ninja: - raise unittest.SkipTest('Rust is only supported with ninja currently') + raise SkipTest('Rust is only supported with ninja currently') testdir = os.path.join(self.rust_test_dir, '12 bindgen') env = get_fake_env(testdir, self.builddir, self.prefix) cc = detect_c_compiler(env, MachineChoice.HOST) @@ -5643,7 +5645,7 @@ class AllPlatformTests(BasePlatformTests): elif cc.get_id() == 'msvc': bad_arg = '/fastfail' else: - raise unittest.SkipTest('Test only supports GCC and MSVC') + raise SkipTest('Test only supports GCC and MSVC') self.init(testdir, extra_args=[f"-Dc_args=['-DCMD_ARG', '{bad_arg}']"]) intro = self.introspect(['--targets']) for i in intro: diff --git a/unittests/darwintests.py b/unittests/darwintests.py index 7b3116853..ddb2c1a3f 100644 --- a/unittests/darwintests.py +++ b/unittests/darwintests.py @@ -1,11 +1,14 @@ # SPDX-License-Identifier: Apache-2.0 # Copyright 2016-2021 The Meson development team +from __future__ import annotations + import subprocess import re import os import platform import unittest +import typing as T from mesonbuild.mesonlib import ( MachineChoice, is_osx, version_compare @@ -20,7 +23,7 @@ from run_tests import ( ) from .baseplatformtests import BasePlatformTests -from .helpers import * +from .helpers import skip_if_not_language, skipIfNoPkgconfig @unittest.skipUnless(is_osx(), "requires Darwin") class DarwinTests(BasePlatformTests): diff --git a/unittests/datatests.py b/unittests/datatests.py index e24e0628e..c531dc358 100644 --- a/unittests/datatests.py +++ b/unittests/datatests.py @@ -1,6 +1,7 @@ # SPDX-License-Identifier: Apache-2.0 # Copyright 2016-2021 The Meson development team +import os import re import unittest from itertools import chain @@ -31,7 +32,7 @@ from run_tests import ( FakeBuild, get_fake_env ) -from .helpers import * +from .helpers import is_tarball @unittest.skipIf(is_tarball(), 'Skipping because this is a tarball release') class DataTests(unittest.TestCase): diff --git a/unittests/failuretests.py b/unittests/failuretests.py index a7e7e1da3..f40771482 100644 --- a/unittests/failuretests.py +++ b/unittests/failuretests.py @@ -25,7 +25,7 @@ from run_tests import ( ) from .baseplatformtests import BasePlatformTests -from .helpers import * +from .helpers import skipIfNoPkgconfigDep @contextmanager def no_pkgconfig(): diff --git a/unittests/internaltests.py b/unittests/internaltests.py index f5d0c9f2c..de4bac48d 100644 --- a/unittests/internaltests.py +++ b/unittests/internaltests.py @@ -53,7 +53,7 @@ from mesonbuild import utils from run_tests import get_fake_env, get_fake_options -from .helpers import * +from .helpers import IS_CI, chdir, skipIfNoPkgconfig class InternalTests(unittest.TestCase): @@ -1080,7 +1080,6 @@ Thread model: posix'''), '21.9.0') return with tempfile.TemporaryDirectory() as tmpdir: - pkgbin = ExternalProgram('pkg-config', command=['pkg-config'], silent=True) env = get_fake_env() compiler = detect_c_compiler(env, MachineChoice.HOST) env.coredata.compilers.host = {'c': compiler} diff --git a/unittests/linuxcrosstests.py b/unittests/linuxcrosstests.py index 0871b59ef..6f7b01760 100644 --- a/unittests/linuxcrosstests.py +++ b/unittests/linuxcrosstests.py @@ -15,7 +15,9 @@ from mesonbuild.mesonlib import MesonException from .baseplatformtests import BasePlatformTests -from .helpers import * +from .helpers import ( + skipIfNoPkgconfig, skipIfNoExecutable +) class BaseLinuxCrossTests(BasePlatformTests): # Don't pass --libdir when cross-compiling. We have tests that diff --git a/unittests/linuxliketests.py b/unittests/linuxliketests.py index 06de99013..80822d77b 100644 --- a/unittests/linuxliketests.py +++ b/unittests/linuxliketests.py @@ -50,7 +50,15 @@ from run_tests import ( ) from .baseplatformtests import BasePlatformTests -from .helpers import * +from .helpers import ( + skip_if_not_language, skip_if_not_base_option, get_rpath, + skipIfNoExecutable, skipIfNoPkgconfig, skipIfNoPkgconfigDep, + chdir, get_soname +) + +if T.TYPE_CHECKING: + from mesonbuild.compilers import Compiler + def _prepend_pkg_config_path(path: str) -> str: """Prepend a string value to pkg_config_path diff --git a/unittests/machinefiletests.py b/unittests/machinefiletests.py index b20b8172f..818409df8 100644 --- a/unittests/machinefiletests.py +++ b/unittests/machinefiletests.py @@ -41,7 +41,9 @@ from run_tests import ( ) from .baseplatformtests import BasePlatformTests -from .helpers import * +from .helpers import ( + skip_if_not_language, skipIfNoExecutable, get_classpath, skip_if_env_set +) @functools.lru_cache() def is_real_gnu_compiler(path): @@ -314,7 +316,7 @@ class NativeFileTests(BasePlatformTests): self.helper_for_compiler('objc', cb) except mesonlib.EnvironmentException as e: if 'GCC was not built with support for objective-c' in str(e): - raise unittest.SkipTest("GCC doesn't support objective-c, test cannot run") + raise SkipTest("GCC doesn't support objective-c, test cannot run") raise @skip_if_not_language('objcpp') @@ -332,7 +334,7 @@ class NativeFileTests(BasePlatformTests): self.helper_for_compiler('objcpp', cb) except mesonlib.EnvironmentException as e: if 'GCC was not built with support for objective-c++' in str(e): - raise unittest.SkipTest("G++ doesn't support objective-c++, test cannot run") + raise SkipTest("G++ doesn't support objective-c++, test cannot run") raise @skip_if_not_language('d') diff --git a/unittests/optiontests.py b/unittests/optiontests.py index 3af71b67c..0b7cf7d2b 100644 --- a/unittests/optiontests.py +++ b/unittests/optiontests.py @@ -1,8 +1,12 @@ # SPDX-License-Identifier: Apache-2.0 # Copyright 2024 Meson project contributors -from mesonbuild.options import * +from mesonbuild.options import ( + OptionStore, OptionKey, UserStringOption, UserStringArrayOption, + UserComboOption, UserBooleanOption, +) from mesonbuild.envconfig import MachineInfo +from mesonbuild.utils.universal import MesonException, MachineChoice import os import unittest @@ -236,7 +240,6 @@ class OptionTests(unittest.TestCase): optstore = OptionStore(False) name = 'cpp_std' sub_name = 'sub' - sub2_name = 'sub2' top_value = 'c++11' aug_value = 'c++23' set_value = 'c++20' diff --git a/unittests/platformagnostictests.py b/unittests/platformagnostictests.py index 7e232af3d..dfd0e6393 100644 --- a/unittests/platformagnostictests.py +++ b/unittests/platformagnostictests.py @@ -15,7 +15,7 @@ from unittest import skipIf, SkipTest from pathlib import Path from .baseplatformtests import BasePlatformTests -from .helpers import * +from .helpers import skip_if_not_language, IS_CI from mesonbuild.mesonlib import EnvironmentVariables, ExecutableSerialisation, MesonException, is_linux, python_command, windows_proof_rmtree from mesonbuild.mformat import Formatter, match_path from mesonbuild.optinterpreter import OptionInterpreter, OptionException @@ -138,7 +138,7 @@ class PlatformAgnosticTests(BasePlatformTests): def check_connectivity(self): import urllib try: - with urllib.request.urlopen('https://wrapdb.mesonbuild.com') as p: + with urllib.request.urlopen('https://wrapdb.mesonbuild.com'): pass except urllib.error.URLError as e: self.skipTest('No internet connectivity: ' + str(e)) @@ -474,7 +474,7 @@ class PlatformAgnosticTests(BasePlatformTests): testdir = self.copy_srcdir(os.path.join(self.common_test_dir, '40 options')) self.init(testdir) self.assertEqual(self.getconf('neg_int_opt'), -3) - with self.assertRaises(subprocess.CalledProcessError) as e: + with self.assertRaises(subprocess.CalledProcessError): self.init(testdir, extra_args=['--reconfigure', '-Dneg_int_opt=0']) self.assertEqual(self.getconf('neg_int_opt'), -3) self.init(testdir, extra_args=['--reconfigure', '-Dneg_int_opt=-2']) diff --git a/unittests/pythontests.py b/unittests/pythontests.py index a24f776cf..20b0f30cd 100644 --- a/unittests/pythontests.py +++ b/unittests/pythontests.py @@ -9,7 +9,7 @@ from run_tests import ( from .allplatformstests import git_init from .baseplatformtests import BasePlatformTests -from .helpers import * +from .helpers import get_fake_env from mesonbuild.compilers.detect import detect_c_compiler from mesonbuild.mesonlib import MachineChoice, TemporaryDirectoryWinProof, is_windows diff --git a/unittests/subprojectscommandtests.py b/unittests/subprojectscommandtests.py index 85950a084..41ba71e95 100644 --- a/unittests/subprojectscommandtests.py +++ b/unittests/subprojectscommandtests.py @@ -15,7 +15,7 @@ from mesonbuild.mesonlib import ( from .baseplatformtests import BasePlatformTests -from .helpers import * +from .helpers import skipIfNoExecutable class SubprojectsCommandTests(BasePlatformTests): def setUp(self): diff --git a/unittests/windowstests.py b/unittests/windowstests.py index 18ea7d38a..2dbcc5133 100644 --- a/unittests/windowstests.py +++ b/unittests/windowstests.py @@ -33,7 +33,9 @@ from run_tests import ( ) from .baseplatformtests import BasePlatformTests -from .helpers import * +from .helpers import ( + get_path_without_cmd, skip_if_not_base_option, IS_CI, skip_if_not_language, +) @skipUnless(is_windows() or is_cygwin(), "requires Windows (or Windows via Cygwin)") class WindowsTests(BasePlatformTests): @@ -459,7 +461,7 @@ class WindowsTests(BasePlatformTests): self.init(testdir, extra_args=['-Dtest-failure=true']) self.assertRaises(subprocess.CalledProcessError, self.build) - @unittest.skipIf(is_cygwin(), "Needs visual studio") + @skipIf(is_cygwin(), "Needs visual studio") def test_vsenv_option(self): if self.backend is not Backend.ninja: raise SkipTest('Only ninja backend is valid for test')