Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanups and Speedup for python unittests #13439

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
136 changes: 58 additions & 78 deletions unittests/allplatformstests.py
Original file line number Diff line number Diff line change
Expand Up @@ -1463,23 +1463,14 @@ def test_dist_git_script(self):
if self.backend is not Backend.ninja:
raise SkipTest('Dist is only supported with Ninja')

try:
with tempfile.TemporaryDirectory() as tmpdir:
project_dir = os.path.join(tmpdir, 'a')
shutil.copytree(os.path.join(self.unit_test_dir, '35 dist script'),
project_dir)
git_init(project_dir)
self.init(project_dir)
self.build('dist')
project_dir = self.copy_srcdir(os.path.join(self.unit_test_dir, '35 dist script'))
git_init(project_dir)
self.init(project_dir)
self.build('dist')

self.new_builddir()
self.init(project_dir, extra_args=['-Dsub:broken_dist_script=false'])
self._run(self.meson_command + ['dist', '--include-subprojects'], workdir=self.builddir)
except PermissionError:
# When run under Windows CI, something (virus scanner?)
# holds on to the git files so cleaning up the dir
# fails sometimes.
pass
self.new_builddir()
self.init(project_dir, extra_args=['-Dsub:broken_dist_script=false'])
self._run(self.meson_command + ['dist', '--include-subprojects'], workdir=self.builddir)

def create_dummy_subproject(self, project_dir, name):
path = os.path.join(project_dir, 'subprojects', name)
Expand Down Expand Up @@ -1538,26 +1529,26 @@ def dist_impl(self, vcs_init, vcs_add_all=None, include_subprojects=True):
self.assertPathDoesNotExist(zip_checksumfile)
# update a source file timestamp; dist should succeed anyway
os.utime(distexe_c)
self.addCleanup(windows_proof_rm, bz_distfile)
self.addCleanup(windows_proof_rm, bz_checksumfile)
self._run(self.meson_command + ['dist', '--formats', 'bztar'],
workdir=self.builddir)
self.assertPathExists(bz_distfile)
self.assertPathExists(bz_checksumfile)
self.addCleanup(windows_proof_rm, gz_distfile)
self.addCleanup(windows_proof_rm, gz_checksumfile)
self._run(self.meson_command + ['dist', '--formats', 'gztar'],
workdir=self.builddir)
self.assertPathExists(gz_distfile)
self.assertPathExists(gz_checksumfile)
self.addCleanup(windows_proof_rm, zip_distfile)
self.addCleanup(windows_proof_rm, zip_checksumfile)
self._run(self.meson_command + ['dist', '--formats', 'zip'],
workdir=self.builddir)
self.assertPathExists(zip_distfile)
self.assertPathExists(zip_checksumfile)
os.remove(xz_distfile)
os.remove(xz_checksumfile)
os.remove(bz_distfile)
os.remove(bz_checksumfile)
os.remove(gz_distfile)
os.remove(gz_checksumfile)
os.remove(zip_distfile)
os.remove(zip_checksumfile)
self.addCleanup(windows_proof_rm, xz_distfile)
self.addCleanup(windows_proof_rm, xz_checksumfile)
self._run(self.meson_command + ['dist', '--formats', 'xztar,bztar,gztar,zip'],
workdir=self.builddir)
self.assertPathExists(xz_distfile)
Expand Down Expand Up @@ -1601,6 +1592,8 @@ def dist_impl(self, vcs_init, vcs_add_all=None, include_subprojects=True):
subproject_dir = os.path.join(project_dir, 'subprojects', 'samerepo')
self.new_builddir()
self.init(subproject_dir)
self.addCleanup(windows_proof_rm, xz_distfile)
self.addCleanup(windows_proof_rm, xz_checksumfile)
self.build('dist')
xz_distfile = os.path.join(self.distdir, 'samerepo-1.0.tar.xz')
xz_checksumfile = xz_distfile + '.sha256sum'
Expand Down Expand Up @@ -1701,6 +1694,7 @@ def pbcompile(self, compiler, source, objectfile, extra_args=None):
cmd += ['/nologo', '/Fo' + objectfile, '/c', source] + extra_args
else:
cmd += ['-c', source, '-o', objectfile] + extra_args
self.addCleanup(windows_proof_rm, objectfile)
subprocess.check_call(cmd, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)

def test_prebuilt_object(self):
Expand All @@ -1709,12 +1703,9 @@ def test_prebuilt_object(self):
source = os.path.join(tdir, 'source.c')
objectfile = os.path.join(tdir, 'prebuilt.' + object_suffix)
self.pbcompile(compiler, source, objectfile)
try:
self.init(tdir)
self.build()
self.run_tests()
finally:
os.unlink(objectfile)
self.init(tdir)
self.build()
self.run_tests()

def build_static_lib(self, compiler, linker, source, objectfile, outfile, extra_args=None):
if extra_args is None:
Expand All @@ -1725,10 +1716,8 @@ def build_static_lib(self, compiler, linker, source, objectfile, outfile, extra_
link_cmd += linker.get_output_args(outfile)
link_cmd += [objectfile]
self.pbcompile(compiler, source, objectfile, extra_args=extra_args)
try:
subprocess.check_call(link_cmd)
finally:
os.unlink(objectfile)
self.addCleanup(windows_proof_rm, outfile)
subprocess.check_call(link_cmd)

def test_prebuilt_static_lib(self):
(cc, stlinker, object_suffix, _) = self.detect_prebuild_env()
Expand All @@ -1738,12 +1727,9 @@ def test_prebuilt_static_lib(self):
stlibfile = os.path.join(tdir, 'libdir/libbest.a')
self.build_static_lib(cc, stlinker, source, objectfile, stlibfile)
# Run the test
try:
self.init(tdir)
self.build()
self.run_tests()
finally:
os.unlink(stlibfile)
self.init(tdir)
self.build()
self.run_tests()

def build_shared_lib(self, compiler, source, objectfile, outfile, impfile, extra_args=None):
if extra_args is None:
Expand All @@ -1759,10 +1745,10 @@ def build_shared_lib(self, compiler, source, objectfile, outfile, impfile, extra
if not is_osx():
link_cmd += ['-Wl,-soname=' + os.path.basename(outfile)]
self.pbcompile(compiler, source, objectfile, extra_args=extra_args)
try:
subprocess.check_call(link_cmd)
finally:
os.unlink(objectfile)
self.addCleanup(windows_proof_rm, outfile)
if compiler.get_argument_syntax() == 'msvc':
self.addCleanup(windows_proof_rm, impfile)
subprocess.check_call(link_cmd)

def test_prebuilt_shared_lib(self):
(cc, _, object_suffix, shared_suffix) = self.detect_prebuild_env()
Expand All @@ -1786,8 +1772,6 @@ def cleanup() -> None:
if os.path.splitext(fname)[1] not in {'.c', '.h'}:
os.unlink(fname)
self.addCleanup(cleanup)
else:
self.addCleanup(os.unlink, shlibfile)

# Run the test
self.init(tdir)
Expand Down Expand Up @@ -2001,8 +1985,6 @@ def test_pkgconfig_static(self):
self.build()
self.run_tests()
finally:
os.unlink(stlibfile)
os.unlink(shlibfile)
if is_windows():
# Clean up all the garbage MSVC writes in the
# source tree.
Expand Down Expand Up @@ -2197,8 +2179,8 @@ def test_options_with_choices_changing(self) -> None:

# Test that old options are changed to the new defaults if they are not valid
real_options = str(testdir / 'meson_options.txt')
self.addCleanup(os.unlink, real_options)

self.addCleanup(windows_proof_rm, real_options)
shutil.copy(options1, real_options)
self.init(str(testdir))
self.mac_ci_delay()
Expand Down Expand Up @@ -3016,6 +2998,8 @@ def test_introspect_projectinfo_without_configured_build(self):

def test_introspect_projectinfo_subprojects(self):
testdir = os.path.join(self.common_test_dir, '98 subproject subdir')
self.addCleanup(windows_proof_rmtree, os.path.join(testdir, 'subprojects/subsubsub-1.0'))
self.addCleanup(windows_proof_rm, os.path.join(testdir, 'subprojects/subsubsub.wrap'))
self.init(testdir)
res = self.introspect('--projectinfo')
expected = {
Expand Down Expand Up @@ -3106,35 +3090,30 @@ def test_clang_format(self):
badheader = os.path.join(testdir, 'header_orig_h')
goodheader = os.path.join(testdir, 'header_expected_h')
includefile = os.path.join(testdir, '.clang-format-include')
try:
shutil.copyfile(badfile, testfile)
shutil.copyfile(badheader, testheader)
self.init(testdir)
self.assertNotEqual(Path(testfile).read_text(encoding='utf-8'),
Path(goodfile).read_text(encoding='utf-8'))
self.assertNotEqual(Path(testheader).read_text(encoding='utf-8'),
Path(goodheader).read_text(encoding='utf-8'))

# test files are not in git so this should do nothing
self.run_target('clang-format')
self.assertNotEqual(Path(testfile).read_text(encoding='utf-8'),
Path(goodfile).read_text(encoding='utf-8'))
self.assertNotEqual(Path(testheader).read_text(encoding='utf-8'),
Path(goodheader).read_text(encoding='utf-8'))

# Add an include file to reformat everything
with open(includefile, 'w', encoding='utf-8') as f:
f.write('*')
self.run_target('clang-format')
self.assertEqual(Path(testheader).read_text(encoding='utf-8'),
Path(goodheader).read_text(encoding='utf-8'))
finally:
if os.path.exists(testfile):
os.unlink(testfile)
if os.path.exists(testheader):
os.unlink(testheader)
if os.path.exists(includefile):
os.unlink(includefile)
shutil.copyfile(badfile, testfile)
self.addCleanup(windows_proof_rm, testfile)
shutil.copyfile(badheader, testheader)
self.addCleanup(windows_proof_rm, testheader)
self.init(testdir)
self.assertNotEqual(Path(testfile).read_text(encoding='utf-8'),
Path(goodfile).read_text(encoding='utf-8'))
self.assertNotEqual(Path(testheader).read_text(encoding='utf-8'),
Path(goodheader).read_text(encoding='utf-8'))

# test files are not in git so this should do nothing
self.run_target('clang-format')
self.assertNotEqual(Path(testfile).read_text(encoding='utf-8'),
Path(goodfile).read_text(encoding='utf-8'))
self.assertNotEqual(Path(testheader).read_text(encoding='utf-8'),
Path(goodheader).read_text(encoding='utf-8'))

# Add an include file to reformat everything
with open(includefile, 'w', encoding='utf-8') as f:
f.write('*')
self.addCleanup(windows_proof_rm, includefile)
self.run_target('clang-format')
self.assertEqual(Path(testheader).read_text(encoding='utf-8'),
Path(goodheader).read_text(encoding='utf-8'))

@skipIfNoExecutable('clang-tidy')
def test_clang_tidy(self):
Expand Down Expand Up @@ -4955,6 +4934,7 @@ def test_symlinked_subproject(self):
symlinked_subproject = os.path.join(testdir, 'subprojects', 'symlinked_subproject')
if not os.path.exists(subproject_dir):
os.mkdir(subproject_dir)
self.addCleanup(windows_proof_rmtree, subproject_dir)
try:
os.symlink(subproject, symlinked_subproject)
except OSError:
Expand Down
41 changes: 19 additions & 22 deletions unittests/baseplatformtests.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
import mesonbuild.coredata
import mesonbuild.modules.gnome
from mesonbuild.mesonlib import (
is_cygwin, join_args, split_args, windows_proof_rmtree, python_command
is_cygwin, is_windows, join_args, split_args, windows_proof_rmtree, python_command
)
import mesonbuild.modules.pkgconfig

Expand Down Expand Up @@ -80,13 +80,19 @@ def setUpClass(cls) -> None:
cls.objcpp_test_dir = os.path.join(src_root, 'test cases/objcpp')
cls.darwin_test_dir = os.path.join(src_root, 'test cases/darwin')

# Keep build directories inside the source tree on Windows and Cygwin so
# that virus scanners don't complain. On sensible OSes put it in the
# the temporary directory
force_tmpdir = os.environ.get('MESON_UNIT_TEST_FORCE_TMPDIR', '0') == '1'
cls._build_dir_root = None if force_tmpdir or not (is_windows() or is_cygwin()) else os.getcwd()

# Misc stuff
if cls.backend is Backend.ninja:
cls.no_rebuild_stdout = ['ninja: no work to do.', 'samu: nothing to do']
cls.no_rebuild_stdout = {'ninja: no work to do.', 'samu: nothing to do'}
else:
# VS doesn't have a stable output when no changes are done
# XCode backend is untested with unit tests, help welcome!
cls.no_rebuild_stdout = [f'UNKNOWN BACKEND {cls.backend.name!r}']
cls.no_rebuild_stdout = {f'UNKNOWN BACKEND {cls.backend.name!r}'}
os.environ['COLUMNS'] = '80'
os.environ['PYTHONIOENCODING'] = 'utf8'

Expand All @@ -96,34 +102,26 @@ def setUp(self):
self.meson_cross_files = []
self.new_builddir()

@property
def privatedir(self) -> str:
return os.path.join(self.builddir, 'meson-private')

@property
def distdir(self) -> str:
return os.path.join(self.builddir, 'meson-dist')

def change_builddir(self, newdir):
self.builddir = newdir
self.privatedir = os.path.join(self.builddir, 'meson-private')
self.logdir = os.path.join(self.builddir, 'meson-logs')
self.installdir = os.path.join(self.builddir, 'install')
self.distdir = os.path.join(self.builddir, 'meson-dist')
self.mtest_command = self.meson_command + ['test', '-C', self.builddir]
if os.path.islink(newdir):
self.addCleanup(os.unlink, self.builddir)
else:
self.addCleanup(windows_proof_rmtree, self.builddir)

def new_builddir(self):
# Keep builddirs inside the source tree so that virus scanners
# don't complain
newdir = tempfile.mkdtemp(dir=os.getcwd())
# In case the directory is inside a symlinked directory, find the real
# path otherwise we might not find the srcdir from inside the builddir.
newdir = os.path.realpath(newdir)
self.change_builddir(newdir)

def new_builddir_in_tempdir(self):
# Can't keep the builddir inside the source tree for the umask tests:
# https://github.com/mesonbuild/meson/pull/5546#issuecomment-509666523
# And we can't do this for all tests because it causes the path to be
# a short-path which breaks other tests:
# https://github.com/mesonbuild/meson/pull/9497
newdir = tempfile.mkdtemp()
def new_builddir(self) -> None:
newdir = tempfile.mkdtemp(dir=self._build_dir_root)
# In case the directory is inside a symlinked directory, find the real
# path otherwise we might not find the srcdir from inside the builddir.
newdir = os.path.realpath(newdir)
Expand Down Expand Up @@ -207,7 +205,6 @@ def init(self, srcdir, *,
args += ['--native-file', f]
for f in self.meson_cross_files:
args += ['--cross-file', f]
self.privatedir = os.path.join(self.builddir, 'meson-private')
if inprocess:
try:
returncode, out, err = run_configure_inprocess(['setup'] + self.meson_args + args + extra_args + build_and_src_dir_args, override_envvars)
Expand Down
4 changes: 1 addition & 3 deletions unittests/failuretests.py
Original file line number Diff line number Diff line change
Expand Up @@ -268,15 +268,13 @@ def test_subproject_variables(self):
'''
tdir = os.path.join(self.unit_test_dir, '20 subproj dep variables')
stray_file = os.path.join(tdir, 'subprojects/subsubproject.wrap')
if os.path.exists(stray_file):
windows_proof_rm(stray_file)
self.addCleanup(windows_proof_rm, stray_file)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes behaviour. The old code ensured that if the file was already there for whatever reason it would be deleted before the test was run.

Copy link
Member Author

@dcbaker dcbaker Oct 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, it's changing behavior. The current behavior is:

  1. the test creates the file
  2. the test runs, and makes no effort to delete the file
  3. on re-run the test deletes the file
    This is weird, and not something that we generally do.

This changes the behavior to match what basically all tests do, and what should happen:

  1. the test creates the file
  2. the test registers a callback to ensure that the file is cleaned up when the test exits, regardless of whether that is in error or in success
  3. on re-run the test doesn't need to look for the file, because it doesn't exist.

We have tons of tests that make the assumption "files I create as part of my test process do no pre-exist", and will die horribly, or worse, fail silently if they do.

out = self.init(tdir, inprocess=True)
self.assertRegex(out, r"Neither a subproject directory nor a .*nosubproj.wrap.* file was found")
self.assertRegex(out, r'Function does not take positional arguments.')
self.assertRegex(out, r'Dependency .*somenotfounddep.* from subproject .*subprojects/somesubproj.* found: .*NO.*')
self.assertRegex(out, r'Dependency .*zlibproxy.* from subproject .*subprojects.*somesubproj.* found: .*YES.*')
self.assertRegex(out, r'Missing key .*source_filename.* in subsubproject.wrap')
windows_proof_rm(stray_file)

def test_exception_exit_status(self):
'''
Expand Down
4 changes: 2 additions & 2 deletions unittests/linuxcrosstests.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class BaseLinuxCrossTests(BasePlatformTests):
def should_run_cross_arm_tests():
return shutil.which('arm-linux-gnueabihf-gcc') and not platform.machine().lower().startswith('arm')

@unittest.skipUnless(not is_windows() and should_run_cross_arm_tests(), "requires ability to cross compile to ARM")
@unittest.skipIf(is_windows() or not should_run_cross_arm_tests(), "requires ability to cross compile to ARM")
class LinuxCrossArmTests(BaseLinuxCrossTests):
'''
Tests that cross-compilation to Linux/ARM works
Expand Down Expand Up @@ -118,7 +118,7 @@ def test_run_native_test(self):
def should_run_cross_mingw_tests():
return shutil.which('x86_64-w64-mingw32-gcc') and not (is_windows() or is_cygwin())

@unittest.skipUnless(not is_windows() and should_run_cross_mingw_tests(), "requires ability to cross compile with MinGW")
@unittest.skipUnless(should_run_cross_mingw_tests(), "requires ability to cross compile with MinGW")
class LinuxCrossMingwTests(BaseLinuxCrossTests):
'''
Tests that cross-compilation to Windows/MinGW works
Expand Down
Loading
Loading