[roottest] dismiss RootExeOptions and RootExternalIncludes cmake variables
#19086
Conversation
It is more clear to provide root.exe options in ROOTTEST_ADD_TEST macro
Use (void)gSystem->Load(\"libbase\") to exclude extra printout Configure fixture instead of dependency
Let configure fixtures and use common dependency scheme
Use (void) to suppress output when loading libs Remove unused ref file
Allows to configure fixture
Adjust ref files while in many tests libEvent library loading not required and therefore extra output caused by loading not created
Use ROOTTEST_ADD_TEST to create and copy hsimple.root files One can set fixtures and correctly organize dependencies Copy ROOT files for runtreeCloneTest2.C directly, let run test on Windows - where PRECMD may lead to test disabling Use fixtures for dependnecies
It replaced by ARG_ROOTEXE_OPTS, which can be configured individual for each test
Instead using RootExternalIncludes variable one can just use ROOTEXE_OPTS which does the same
No need for special variable only for includes Build library with ROOTTEST_GENERATE_DICTIONARY Configure dependency with fixtures
Setup correct fixture for ROOT-10426 test
Replaced by ROOTEXE_OPTS argument which is individual for each test and does not interfere with other tests
| ROOTTEST_ADD_TEST(ROOT-10426 | ||
| MACRO execROOT10426.C | ||
| OUTREF execROOT10426.ref) | ||
| add_test(NAME roottest-cling-staticinit-ROOT-10426-lib |
There was a problem hiding this comment.
Is this needed? Isn't the library build during the main ROOT build?
There was a problem hiding this comment.
But idea not to depend from ROOT build.
| ROOT_GENERATE_DICTIONARY(G__CmsPair ${CMAKE_CURRENT_SOURCE_DIR}/cmspair.h LINKDEF PairLinkDef.h) | ||
| ROOTTEST_LINKER_LIBRARY(CmsPair TEST G__CmsPair.cxx LIBRARIES ${ROOT_LIBRARIES}) | ||
|
|
||
| ROOT_ADD_TEST(roottest-root-io-stdpair-collection-build |
There was a problem hiding this comment.
Changing this may or may not be the right direction. However it 'looks' like a noticeable change (see also root-project/roottest#858) that does not appear directly link to RootExeOptions. Can you open a separate PR for this? Thanks.
There was a problem hiding this comment.
Changing RootExeOptions is trivial in most cases.
But now split them will be too much work.
There was a problem hiding this comment.
Ok. Bear with me while I look at this one directory a bit longer.
| testobject.h testobjectderived.h | ||
| LINKDEF baseLinkDef.h | ||
| SOURCES testobject.cpp testobjectderived.cpp | ||
| NO_CXXMODULE |
There was a problem hiding this comment.
This seems to be a behavior change (vs MODULE base). Why is it advantageous?
There was a problem hiding this comment.
I had failures on some platforms - because of auto loading with hadd
There was a problem hiding this comment.
We should add a comment here of what those failures are and why they require the no cxxmodule. However I also don't see immediately why it worked before and now fails.
| @@ -1,5 +1,4 @@ | |||
|
|
|||
| (int) 0 | |||
There was a problem hiding this comment.
I am vaguely surprised those disappeared ... (this seems related to the comment at line 7 of roottest/root/io/transient/base/CMakeLists.txt)
| ROOTTEST_ADD_TEST(hsimple-create | ||
| MACRO ${CMAKE_SOURCE_DIR}/tutorials/hsimple.C | ||
| PASSRC 255 | ||
| FIXTURES_SETUP root-tree-cloning-hsimple-fixture) |
There was a problem hiding this comment.
This is now technically "redundant" with the last step of the ROOT build (i.e. we are now always in the else branch (line 40 of the previous version of this file.
Test Results 15 files 15 suites 2d 21h 18m 59s ⏱️ For more details on these failures, see this check. Results for commit fcfd04b. |
Both variables adding custom arguments to
root.exeinvocation in the roottest macros.Main disadvantage of such variables - all consequent tests will use such variables -
which was not necessary in several cases, especially in
root/tree/cloningtests.All this can be replaced by
ROOTEXE_OPTSargument ofROOTTEST_ADD_TESTmacro.It assigned per-test and therefore more precise.
In the tests where such options were replaced also fix dependencies using fixtures -
which is more reliable than use of
DEPENDSarguments.