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

Test target depends on all executables #1704

Closed
gsauthof opened this issue Apr 29, 2017 · 10 comments
Closed

Test target depends on all executables #1704

gsauthof opened this issue Apr 29, 2017 · 10 comments

Comments

@gsauthof
Copy link

Say I have a meson build that defines several executables including a unittest-suite executable. Then I define one test for the one unittest executable.

Example:

executable('some_frontend', 'foo_etc.cc')
executable('some_frontend2', 'foo_etc2.cc')
# some more executable definitions
ut = executable('ut', 'some_sources.cc')
test('unittest', ut)

When I execute:

$ ninja-build ut
$ ninja-build test

Expected behavior: The ninja-build test just executes the unittest executable since the test only depends on that executable.

Actual behavior: The ninja-build test also builds all other executables that are defined in the meson build file.

This is caused because meson generates the ninja test target like this:

build test: CUSTOM_COMMAND all PHONY

The expected behavior would require the generation of something like:

build test: CUSTOM_COMMAND ut PHONY
@nirbheek
Copy link
Member

This is indeed how make check works with Autotools, and we should probably make it work like this. However, we have to think about how to do this because this will break people's setups.

@jpakkane
Copy link
Member

No. The current behaviour is there by design and will not be changed. Ever! In Meson "ninja test" always means running all tests with everything up to date. If I had an euro for every case where people have run tests without everything being up to date because "I know that that I only need to build X for this" and then failing spectacularly I could spend the rest of my life under a very comfortable tree in the Bahamas.

That being said what we can do, and AFAICR Nirbheek has already looked into this, is to make this possible with Mesontest. So you could do the equivalent of "mesontest --only-rebuild-what-is-needed testname" which would bring that target and everything it depends on up to date. This is still dangerous and has failure potential, but since it is explicit, people are prepared for weird issues.

@nirbheek
Copy link
Member

If I had an euro for every case where people have run tests without everything being up to date because "I know that that I only need to build X for this" and then failing spectacularly I could spend the rest of my life under a very comfortable tree in the Bahamas.

The whole point of Meson is that you can actually ensure that this will not happen. What's the point of tracking all dependencies carefully if we just throw all that information away and build all the targets before running tests?

So you could do the equivalent of "mesontest --only-rebuild-what-is-needed testname" which would bring that target and everything it depends on up to date. This is still dangerous and has failure potential, but since it is explicit, people are prepared for weird issues.

Honestly, while running specific tests, we should definitely not build everything that is out of date (like we do right now). It is a total and complete waste of time in larger projects. In fact, the decision to always build everything has caused people to not bother with properly specifying dependencies in test() and made the transition to sanity worse.

@jpakkane
Copy link
Member

The whole point of Meson is that you can actually ensure that this will not happen.

This can not be detected reliably. If you have a test that does internal file system globbing or something else it has a secret dependency. Or it may call a built executable that is linked against some shared library that is updated but the other executable is not relinked because it was not listed as a dependency of the test. Or any of the myriad of ways to have a non-obvious or hidden dependency. The only reliable way is to make sure everything is up to date.

@nirbheek
Copy link
Member

If you have a test that does internal file system globbing or something else it has a secret dependency.

custom targets can do that too, and yet we handle those just fine with keyword arguments.

Or it may call a built executable that is linked against some shared library that is updated but the other executable is not relinked because it was not listed as a dependency of the test.

Same thing with custom targets, and yet we handle those fine.

Or any of the myriad of ways to have a non-obvious or hidden dependency.

All those are handled just fine with every other meson function!

The only reliable way is to make sure everything is up to date.

Yes, let's give up and always rebuild everything for every target including custom targets. It's the only way to be absolutely sure.

Tests are no different from custom targets or run targets, and there is no reason to treat them specially, especially given how many people expect them to not behave specially. People are going to keep filing bug reports about this till we change it.

@jpakkane
Copy link
Member

There are two different issues here. The first one is adding functionality so people can run a subset of tests and we only rebuild those targets that are explicitly mentioned (the exe, its direct dependencies, other targets given on the command line, extra dependency targets). This is good and useful functionality, which we can add to mesontest, no doubt about that. We might even add something like mesontest --minimal-rebuild which does exactly what the original bug message requested.

The second issue is what should ninja test do by default. The current semantics are "make sure everything is up to date and run tests". This is a good default because it is the reliable thing to do.

If sometime in the future we change the default from current to minimal rebuilds (which I consider very unlikely) having it in the mesontest command as an optional thing is good because it allows people to work out the kinks in an opt-in fashion.

@nirbheek
Copy link
Member

I find this position completely reasonable. Thanks for discussing this with me!

@gsauthof
Copy link
Author

FWIW,

  • building everything when having coverage data collection enabled also introduces noise in the baseline that you have to filter out again
  • the test target generated by cmake/ctest also doesn't depend on all executables
  • I haven't met anyone so far who complained or was burned by the expected behavior I've described
  • it is pretty standard to run tests in a clean continuous integration environment (e.g. in a newly instantiated container - like with travis CI) - thus when you really miss to add a dependency on needed additional executables your test just fails

In any case, perhaps you want to update the test() section the reference manual such that the current design is documented.

@jpakkane
Copy link
Member

I haven't met anyone so far who [...] was burned by the expected behavior I've described

I have, that is the main reason the current behavior is what it is.

@bonzini
Copy link
Collaborator

bonzini commented Dec 31, 2024

This is intentional as "ninja test" is a simplified workflow, and I am closing the issue because now the tests track their dependencies in introspection data and meson test uses that when starting individual test. See #7902 and #10413.

@bonzini bonzini closed this as completed Dec 31, 2024
eli-schwartz added a commit to eli-schwartz/meson that referenced this issue Jan 8, 2025
When running `ninja all` we shouldn't build testsuite programs as these
might not be wanted e.g. in order to just install the project. We do
want them to be built when running `ninja test`. Since meson 0.63 we
actually have a dedicated ninja alias for test dependencies -- move
these from the "all" rule to the dedicated test/benchmark rules.

Closes: mesonbuild#1704
Closes: mesonbuild#1949
Closes: mesonbuild#2518
Closes: mesonbuild#3662
Closes: mesonbuild#5728
Closes: mesonbuild#5867
Closes: mesonbuild#6511
Closes: mesonbuild#11317
Closes: mesonbuild#13378
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants