add BUILD_TESTING=OFF to presets if tools.build:skip_test

[memsharded]

Changelog: Feature: Add BUILD_TESTING=OFF to CMakeToolchain presets if tools.build:skip_test.
Docs: conan-io/docs#2713

Close #11905

[jcar87]

This looks good but I have a question!
tools.build:skip_test is documented as:

Do not execute CMake.test() and Meson.test() when enabled

In this PR, we would be a going a step beyond that - and potentially not building tests at all (depending on whether or not the projects use BUILD_TESTING to control the building of the tests.

This can be convenient as on some projects this can save considerable compilation time, but on the other hand in some projects the tests are the first attempt at linking things that consume the library and it can serve as an early way of catching linking issues. Although it could be argued that is the purpose of the test_package anyway.

[memsharded]

Yes, I would say that the expected behavior of the Conan integrations, regarding skip_test is to avoid building them too, if they are not going to be ran anyway. I think the docs can be updated to reflect this. As always, this is only the default, and projects listening to BUILD_TESTING should behave correctly for both True and False values, and can define themselves in the recipe a different logic to tc.cache_variables["BUILD_TESTING"] = xxxx if they want to, and that will be respected.

[franramirez688]

I think that this is the expected behavior, and the user will save time whenever he does not want to run all the tests.

[uilianries]

What about CMakeToolchain.variables? Is it a possible scenario, right?

[memsharded]

It sounds like CMakeToolchain.variables should be for user variables, while everything that is a CMake built-in, should be to cache_variables otherwise there is a risk that the variables will not have enough priority. @jcar87 wdyt?

[jcar87]

I'd say that's a good approximation!

I can find widespread evidence on GitHub that BUILD_TESTING is treated as a cache variable by projects:

1. For example here by calling option()

2. It will also be a cache variable if the user provides it via -DBUILD_TESTING=ON/OFF - in this case, the user's preference will take precedence and be respected regardless of whether we define it as a cache_variable or the project sets an option() -

So I think the way it currently is in this PR is the safest and least likely to have side effects - especially if there's any chance projects define or reference BUILD_TESTING before the first call to project() (have found examples here and here)

[memsharded]

Ok, lets recommend that in the docs then 👍

[uilianries]

I'm not in the opposite of using cached_variables, but still, users are free to use both and it will have a different behavior, depending the usage.

[memsharded]

Yes, but if we explicitly say it in the docs, they should use each one for the documented cases, so they will not be "free" to use them invariantly for all cases.