Question about Axel's news posts (Tests and Argparse)

On the main ROOT page, two news posts by @Axel are listed on top:

I’ve got questions to both of them.

About testing:

In ROOT-9990, I added a test case for invalid formulae. However, these tests reside in the test directory of the main ROOT repository, not in the separate roottest repository mentioned in the post.
In the case of TFormula, now there are 51 tests (functions called “test1” to “test51”) in test/TFormulaParsingTests.h in the root repo, and further tests are in the root/hist/formula/ subdirectry of the roottest repository. The latter seems to contain .C file invocations while the former seem to contain more unittest style tests. I added the test to TFormulaParsingTests.h (seemed to make more sense there, and yes, it’s a header file containing non-header code), but it is not really clear to me where to add further tests. Also, is there any preferred testing framework already in use (like Google Test or Catch2)? I find it a bit weird and error prone to count the number of tests manually and to “&&” the tests results inside of the testNN functions. So, tl;dr: a) which test framework to use (if any) and b) where to put the tests?

About command line parsing

I thought of the framework to use. Boost’s program_options came to mind (just because it is the first one I’ve used, not because of preference - I don’t have a strong opinion here). Is it okay to depend on Boost, or should one go for other libraries such as cxxopts or args? The non-boost libs might be smaller & easier to use, but with Boost there might be better long-term guarantees. And if an external lib is included, should it be downloaded during the build process, or should a specific version be copied into the ROOT repo? It seems to me more complicated to get these decisions right than to update the actual code once a decision has been made.

Hi @behrenhoff,
the policy is that small tests/unit tests/quick tests should be part of the main root repo in the test directory in the appropriate module (e.g. TTree “unit” tests are in $ROOTSYS/tree/tree/test).

We use googletest for these tests, e.g. open any test in $ROOTSYS/tree/dataframe/test and you’ll see plenty of EXPECT_EQ.

Regarding the library of choice for command line parsing, I think boost has been a no-go historically, but I’d better let @Axel comment further.

Cheers,
Enrico

@behrenhoff , my favorite c++14 header only library (by far) is clipp and I find myself using it in more and more projects. It is very easy to quickly develop a program with a well documented and robust command line interface.

Check it out https://github.com/muellan/clipp#examples

This topic was automatically closed 14 days after the last reply. New replies are no longer allowed.

We don’t want to depend on boost; a header-only “library” that we can (possibly optionally) include in ROOT’s sources is fine. That means it should be LGPL’ed or MIT’ed.

Note that ROOT has command line parameters that violate assumptions of most argparsers, e.g. -memstat (single dash but long arg), just like rootcling which has (arguably even worse) -split and -s

Cheers, Axel.

Thanks for reopening this topic.

Note that ROOT has command line parameters that violate assumptions of most argparsers, e.g. -memstat
single dash but long arg), just like rootcling which has (arguably even worse) -split and -s…

First a disclaimer: I wanted to try whit2333’s suggestions since I am using boost::program_options most of the time but don’t like it very much. So why not have a look at ROOT?

Parsing -s -split and even combining -b -q to -bq or -qb at the same time is not at all a problem with clipp. I just had a look at TApplication.cxx last week and the really challenging aspects are to understand what ROOT is doing at the moment when using the “non-dash” options (directory, file.root, file.C, or --)

For example, I learnt that arguments after -- are passed as arguments to the last listed macro. That means the following commands do the same:
root -b -q 'macro.C("hello", 42)'
root -b -q macro.C -- '"hello"' 42

root --help doesn’t tell you that this is possible. So does anyone use this? I guess this behaviour must be kept.

Usually -- is taken to separate arguments the rest / from a list of files. I’d have expected to be able to execute a macro called ‘-name.C’ or to open a ‘-name.root’ by running root -- -name.C (or .root respectively) (not that I like file names starting with a dash).

But this: root -q -- -hello.C just prints the help page since anything starting with -h or --help is printing help, and also any parameter starting with --version prints the version. Is it really useful to keep this behaviour or should -helpmepleaseiamfedupwithrootscommandline be disallowd as alias for -h?

Currently, there is more strange behaviour if you pass the macro using a relative path first, then a directory (should this really be allowed?).
Consider PWD=/home/test and a macro /home/test/macro.C

root ./macro.C /tmp
(passes command line validation successfully and tries to run)
Processing ./macro.C…
Error in TApplication::ExecuteFile: macro ./macro.C not found in path .:/opt/root-6.16.00p/macros

cd /tmp
root ./macro.C /home/test
Warning in TApplication::GetOptions: macro ./macro.C not found
(fails while parsing command line)

So if you specify the directory after the macro, you need an absolute path for the macro.

I haven’t checked how or in which order ROOT handles a mix of file.root macro.C file2.root macro2.C ....

Another aspect is that ROOT is changing the used-up argv[i] to "" and decrementing argc accordingly. clipp doesn’t do that - at least I didn’t find an option for it so far. Currently, unknown arguments such as -bq currently stay in the args for further use elsewhere (and thus cannot be shown in the automated help). TRootApplication for example parses just the -display option. I am a little unsure here (for example, how can I test a TRootApplication - which executable does that correspond to?) And shouldn’t “-display” show up in the help text as well - but the command line is currently parsed before gGuiFactory->CreateApplicationImp is called…

As for the progress: I have built a test-executable that tries to mimic ROOT’s cmdline (except for a few missing things). clipp seems simple to use!

Maybe it would have been simpler to start off with rootcling_impl even though RootClingMain is a >1200 line monster function :slight_smile:

I was planning to add named arguments support so one could provide macro arguments out of order and/or omit some of them (if a default arg exists), e.g. call a main macro function void macro(T1 arg1 = 'deflt1', T2 arg2 = 'deflt2', ...) as root macro.C -- arg2='non-deflt', but this was not accepted and what was accepted is just macro.C -- a b cmacro.C(a, b, c) mapping.
It simplifies invocation slightly (no need to escape parenthesis), but actually I find this pretty useless and I’ve never used it.
I much doubt anyone uses it and believe this feature should be removed.

I agree that there is no point in keeping this behavior.

I believe user code might rely on only getting unparsed options after TApplication is done handing its arguments.

If multi-arg handling (-nb) changes what user code is exposed to then so be it - we cannot guarantee backward compatibility for that.

I believe the use case of passing function parameters is more common than passing files starting with -. The mental model behind this is nicely matching what’s in here: shell - What does "--" (double-dash) mean? - Unix & Linux Stack Exchange I.e. it’s not about “file names” but about “and then come some positional arguments that ROOT itself shouldn’t bother with” (because they are passed to the script). We should improve the help!

The macro vs dir behavior is not helpful. Whatever we decide to do (fix when the macro is looked for, or when the cd happens) should be fine and not break existing use cases.

For TRootGuiFactor I prefer to wait for @bellenot - he will be back end of the week.

Thanks a lot for looking into this! And yes, rootcling is way more localized than TApplication so to did start with the more difficult problem…

In case someone finds this thread first: discussion has moved here.