acts-project / detray

Test library for detector surface intersection
Mozilla Public License 2.0
10 stars 18 forks source link

Build Tests by Default #830

Closed beomki-yeo closed 1 month ago

beomki-yeo commented 1 month ago

This is definitely for my own quality of life.

beomki-yeo commented 1 month ago

That is possible, but is there any reason you don't want them built by default? traccc also does the same thing.

The same problem will happen if I start build the project with the tests OFF and try to rebuild them with ON.

stephenswat commented 1 month ago

The problem is that we have to maintain a long list of what to turn off in:

And in any other dependency. So every time you add, remove, or change any of the detray build flags we need to update everything that uses detray. It's much easier if these things are off by default, and then we can turn them on for development.

beomki-yeo commented 1 month ago

You won't have to touch any of them, will you? - The options that I touched in this PR seem to be specified in all links you attached.

beomki-yeo commented 1 month ago

And whether they are ON or OFF in detray is not supposed to be a problem in any case. I believe it is the clients' responsibility to turn off unnecessary part of libraries.

They can blame the library only when it changes the names of options for no good reasons

beomki-yeo commented 1 month ago

Fine. Let me quickly add the Top Level stuff as it seems easy

paulgessinger commented 1 month ago

It's a bit subjective, but imo the library should build only core functionality, and anything else should be off by default.

Aside from this, there's PROJECT_IS_TOP_LEVEL from CMake 3.21

andiwand commented 1 month ago

It's a bit subjective, but imo the library should build only core functionality, and anything else should be off by default.

Aside from this, there's PROJECT_IS_TOP_LEVEL from CMake 3.21

I second that. The default options should not be tuned towards a dev setup but rather a user setup. To make your life easier you can have a script to create a dev setup or use CMake presets as suggested.

beomki-yeo commented 1 month ago

I will use PROJECT_IS_TOP_LEVEL BTW, this is not just about ON and OFF - you need to see #826.

stephenswat commented 1 month ago

Actually, thinking about it PROJECT_IS_TOP_LEVEL also won't work because it will enable the building of tests and benchmarks even if someone is just trying to install detray. :slightly_frowning_face:

beomki-yeo commented 1 month ago

Actually, thinking about it PROJECT_IS_TOP_LEVEL also won't work because it will enable the building of tests and benchmarks even if someone is just trying to install detray. 🙁

You meant a standalone installation?

stephenswat commented 1 month ago

Actually, thinking about it PROJECT_IS_TOP_LEVEL also won't work because it will enable the building of tests and benchmarks even if someone is just trying to install detray. 🙁

You meant a standalone installation?

Yes, exactly.

beomki-yeo commented 1 month ago

I only turned on the UNITTESTS and INTEGRATIONTESTS, so it won't take lots of time for default setup.

We can turn OFF EIGEN and VC as I usually turn off them manually for speedy compilation. @niermann999 how do you think

niermann999 commented 1 month ago

We can turn OFF EIGEN and VC as I usually turn off them manually for speedy compilation. @niermann999 how do you think

Yes, I think we should turn the extra plugins off by default as well

sonarcloud[bot] commented 1 month ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

niermann999 commented 1 month ago

Actually, thinking about it PROJECT_IS_TOP_LEVEL also won't work because it will enable the building of tests and benchmarks even if someone is just trying to install detray. 🙁

You meant a standalone installation?

Yes, exactly.

Has this been solved by #835 @beomki-yeo ?