Skip to content

Disable magic_enum tests#2427

Closed
frankier wants to merge 1 commit intomesonbuild:masterfrom
frankier:magic_enum_tests
Closed

Disable magic_enum tests#2427
frankier wants to merge 1 commit intomesonbuild:masterfrom
frankier:magic_enum_tests

Conversation

@frankier
Copy link

@frankier frankier commented Oct 1, 2025

I think usually anything using this as a dependency won't want these. Additionally, currently the tests bundle a version of catch2 that has problems with Windows.

Copy link
Contributor

@stephanlachnit stephanlachnit left a comment

Choose a reason for hiding this comment

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

This doesn't make any sense here - simply set the corresponding setting in your meson.build files:

magic_enum_dep = dependency('magic_enum', default_options: ['test=false'])

@frankier
Copy link
Author

frankier commented Oct 1, 2025

I have made that change to my meson.build -- I'm trying to help others not have to.

I'm open to the idea that this doesn't make sense, but would it be possible to explain a bit further? Is it normal to run the test suites of your dependencies while building? This is what seems to happen at the moment. I'm thinking this is probably not intentional. Don't the various patches in this repository exist to make things "just work" when adding other projects as dependencies or have I misunderstood the purpose of WrapDB?

Looks like I need to fix up this PR if it turns out it is wanted, but I'll wait for your reply.

@eli-schwartz
Copy link
Member

I'm open to the idea that this doesn't make sense, but would it be possible to explain a bit further? Is it normal to run the test suites of your dependencies while building? This is what seems to happen at the moment. I'm thinking this is probably not intentional.

This is a very personal sort of decision. It is definitely normal for me to run the test suites of my dependencies. The rationale is that I want to know I've correctly built my dependencies, and that my dependencies aren't incompatible, whether explicitly at compile time or subtly at runtime due to buggy assumptions, with either my CPU, or my operating system, or my compiler, etc.

If it was actually correct to never run test suites "when used as a dependency of another project", then we could rewrite that belief into "it is correct to never run test suites more than once. Only upstream should ever run test suites, and then only on code changes". And yes! There are a lot of people who do believe this. There are entire programming language ecosystems (I'm looking at you, python) where the culture is heavily into "testsuite files should be deleted from source code archives to make them a few kilobytes smaller to download, and if anyone says they wanted to run the testsuite, simply shame them into retracting their request".

I do believe very strongly that running tests should be a user choice. I'm strongly supportive of offering build options to permit successfully configuring without tests, which is especially important when tests have third-party dependencies such as catch2. :) But I'm not convinced it makes sense to change the default (especially if it introduces a brand new requirement to patch the build files).

I would also like to note that meson itself has fundamental behaviors that enable this choice of whether or not to run tests of dependencies:

  • meson test can select a "suite" of tests to run, and meson automatically tags every test with, at minimum, a suite based on the (sub)project name, so it's possible to say "run tests for the primary project only" without even knowing whether or not subprojects have any tests
  • test programs can be configured to not take any time when building the project, by using build_by_default: false. They will only be built, in that case, at the time of running meson test (assuming the test hasn't been deselected via suite). This is useful for the primary project as well, by the way. :)

Don't the various patches in this repository exist to make things "just work" when adding other projects as dependencies or have I misunderstood the purpose of WrapDB?

Not necessarily. The WrapDB curently exists for two reasons:

  • to provide *.wrap files for dependencies, even if those dependencies already have well maintained meson.build files
  • to provide a shared location for collaborative maintenance of unofficial meson.build files for any given project that doesn't have (and may not be willing to merge) them, and a single place people can go to find and download those unofficial meson.build files

If upstream does have meson.build files but they aren't suitable for use at all, then the WrapDB can also make them usable. This is for example the case with zstd, which maintains the files upstream but only in a subdirectory, which means zstd cannot be registered as a subproject at all unless a patch is applied to create a root-level meson.build file

'test',
type : 'boolean',
value : false,
description : 'Build and run tests'
Copy link
Member

Choose a reason for hiding this comment

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

From a purely practical perspective, it is very intentional that the WrapDB itself runs the upstream project tests when adding/updating/modifying a wrap.

So if a wrap does disable tests "by default" regardless of why it disables them, it should be using ci_config.json to pass the correct build options to re-enable them inside of the WrapDB itself.

@frankier
Copy link
Author

frankier commented Oct 2, 2025

Thanks a lot for the explanation. As you touched upon, I am much more familiar with standard practices in Python-land but it does make sense that you might want to run tests for dependencies in C++ since environments can vary so wildly. I think maybe this should/could be tackled in magic_env since build_by_default: false seems a bit more standard. Let's see what they say, but in any case I accept your arguments against having this in wrapdb.

@frankier frankier closed this Oct 2, 2025
@eli-schwartz
Copy link
Member

eli-schwartz commented Oct 3, 2025

Thanks a lot for the explanation. As you touched upon, I am much more familiar with standard practices in Python-land but it does make sense that you might want to run tests for dependencies in C++ since environments can vary so wildly.

Nope. :)

I've fought with sometimes great pain and expense to run tests for python code too, including python code that doesn't use any C extensions. It does catch bugs.

The two major reasons for bugs are:

  • testing for a newer version of CPython than is configured to run in upstream CI. Distros often start rebuilding the entire python ecosystem starting with python betas, in order to be ready to migrate to a new python version as soon as possible. e.g. Fedora and Gentoo
  • "exact version pinning of dependencies" goes through phases where people love it and where people describe it as a terrible thing. But in distro land, we always said it was a terrible thing. Holding up the entire world when a package has a minor bugfix release because one package pins the version is... not great. But most projects don't actually test every version of their deps in CI. Testing against the installed version is a very good thing, and it becomes trivial to e.g. rebuild all installed Gentoo packages with tests enabled, to test against the other simultaneously-installed packages. And sometimes that catches issues with the fact that we tried to optimistically upgrade to a newer version of a dependency, and there was a minor incompatibility. ;)

Unfortunately pip doesn't know how to run tests for installed packages, even when building from sdists, because testing in python land is completely non-standardized. In Linux distros we simply don't care whether it is standardized -- we have our own standard hook for running tests so we write out the relevant testing command for each package individually. It is trivial and works great. It remains an eternal disappointment to me that some python developers go to extra lengths to rm -rf the testsuite files from sdists, and when politely asked if it would be possible to ship the testsuite files just in case someone would like to run the tests manually... respond with great hostility, inform us that it "violates the community standard" (???) to ship test files, and that our request is some kind of sneaky attempt at making pip users get slower packages because the individual downloads grow from 200kb to 300kb. They simultaneously fail to note that pip users never see sdists if a wheel exists, and pip users will be more slowed down by running setuptools bdist_wheel on an sdist by many orders of magnitude, than by tests.

</rant>

;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants