[tmva][sofie] Don't compile emitted code for testing at build time #19370
Conversation
Now that the repositories are merged, it makes sense to also make the roottest macros available in all of ROOT, so that they can also be used by tests outside of the roottest directory where convenient.
Use relative path to source dir for roottest names fallback if `ROOTTEST_DIR` is not defined.
guitargeek
added
in:Build System
in:TMVA
clean build
guitargeek
force-pushed
the
build_sofie_tests
branch
from
95a9ffe to
574ed1e
Compare
Test Results 21 files 21 suites 3d 13h 41m 36s ⏱️ For more details on these failures, see this check. Results for commit 574ed1e. |
|
Thanks for the review! You are right, fixtures are clearer than the magic |
Don't compile emitted code for testing at build time, but use the convenient ROOTTEST macros to define the right sequence of emitting code, building, and testing. Closes root-project#18554.
guitargeek
force-pushed
the
build_sofie_tests
branch
from
574ed1e to
e76e651
Compare
There was a problem hiding this comment.
One general comment regarding the change in the build system. Note that even though roottest got merged into root we still have the build option roottest which could be OFF. In that case, the ROOTTEST macros being defined inside RootMacros.cmake might result in missing variables or anything wrong with the cmake. Probably this should be evaluated
|
Good question! I checked to build and run the tests with Also I added one more commit to fix an unused variable warning. |
guitargeek
force-pushed
the
build_sofie_tests
branch
from
c978991 to
556bfcc
Compare
|
|
||
| #------------------------------------------------------------------------------- | ||
| # | ||
| # Former RoottestMacros.cmake starts here |
There was a problem hiding this comment.
I would have found it much clearer to just move the file as is .... even if it was then 'included' here ....
There was a problem hiding this comment.
For the commit history maybe, but my hope was that by bringing these files together, more code duplication will be apparent and inspire code improvements along the way in the future.
Following a discussion with @lmoneta and @sanjibansg.
Don't compile emitted code for testing at build time, but use the convenient ROOTTEST macros to define the right sequence of emitting code, building, and testing.
Closes #18554.
To make this easier, move the
ROOTTESTmacros to the globalRootMacros.cmakefile, which should be fine now that the repositories are merged anyway. What do you think, @pcanal and @linev?