orc icon indicating copy to clipboard operation
orc copied to clipboard

Improve command-line options handling in ToolTest.cc

Open lahwaacz opened this issue 7 months ago • 6 comments

ToolTest.cc has terrible handling of command-line options:

https://github.com/apache/orc/blob/1d33d2aa0ef82aa080d7c9e1b5ac6b814c859f21/tools/test/ToolTest.cc#L37-L58

First, it interprets argv[1] as exampleDirectory and argv[2] as buildDirectory. Then it passes all options to testing::InitGoogleTest(&argc, argv);

This means that passing optional arguments like --gtest_* does not work. If I do

gtest_filter="-TestMatchParam/FileParam.Contents/19:-TestMatchParam/FileParam.Contents/23"
build/tools/test/tool-test --gtest_filter="$gtest_filter"

then all tests except one fail with an error like

C++ exception with description "Can't open --gtest_filter=-TestMatchParam/FileParam.Contents/19:-TestMatchParam/FileParam.Contents/23/TestOrcFile.testDate2038.orc" thrown in the test body.

lahwaacz avatar May 24 '25 11:05 lahwaacz

Any suggestion or working PRs? This is open source community. You can contribute in a way you want to expect to be.

dongjoon-hyun avatar May 24 '25 14:05 dongjoon-hyun

The testing::InitGoogleTest(&argc, argv); should be done first as it will remove gtest options from argv: https://stackoverflow.com/a/4820180

Then see if you need to do anything special about parsing the remaining arguments...

lahwaacz avatar May 24 '25 15:05 lahwaacz

Sure. Since this is an open source project to open to all suggestions, feel free to open a PR if you want to make it in that way. We can continue to discuss on your PR.

The testing::InitGoogleTest(&argc, argv); should be done first

In general, for the proposal about test code, we can easily adopt the new way. I hope the proposed change is concise and small.

dongjoon-hyun avatar May 27 '25 17:05 dongjoon-hyun

Now that we have added Meson support, this manifests itself when running the test suite and using Orc as a subproject, rather than the main project being built. I think this tests ends up resolving the build directory to the main project, and therefore the tests are unable to find the executables.

Here's an example of that from the logs:

https://github.com/mesonbuild/wrapdb/actions/runs/15763207309/job/44434342657?pr=2206#step:4:27298

Will take a look at this in more detail

WillAyd avatar Jun 19 '25 17:06 WillAyd

I wonder if instead of trying to get these argument from the command line, it might be better for the build system to pass along its build directory location and the source location of the examples directory

WillAyd avatar Jun 19 '25 17:06 WillAyd

This should be closed by #2291

WillAyd avatar Jun 25 '25 23:06 WillAyd

Thank you, @WillAyd . Let me close this issue.

dongjoon-hyun avatar Jul 04 '25 23:07 dongjoon-hyun

For the record, Apache ORC 2.2.0 will arrive this month. We are actively working on finalizing it, @lahwaacz .

dongjoon-hyun avatar Jul 04 '25 23:07 dongjoon-hyun