[libcamera-devel] [PATCH v8 5/5] lc-compliance: Add list and filter parameters
Nícolas F. R. A. Prado
nfraprado at collabora.com
Mon Jun 21 21:37:02 CEST 2021
Hi Jacopo,
On Fri, Jun 18, 2021 at 12:09:58AM +0200, Jacopo Mondi wrote:
> Hi Nicolas,
>
> On Wed, Jun 16, 2021 at 10:25:35AM -0300, Nícolas F. R. A. Prado wrote:
> > Add a --list parameter that lists all current tests (by mapping to
> > googletest's --gtest_list_tests).
> >
> > Add a --filter 'filterString' parameter that filters the tests to run
> > (by mapping to googletest's --gtest_filter).
> >
> > Signed-off-by: Nícolas F. R. A. Prado <nfraprado at collabora.com>
> > Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> > ---
> > Changes in v8:
> > - Thanks to Kieran:
> > - Switched from malloc/free to new/delete in gtest parameter allocation
> >
> > Changes in v7:
> > - Thanks to Niklas:
> > - Removed intermediary filter string variable in Harness::initGtestParameters()
> >
> > Changes in v6:
> > - Thanks to Niklas:
> > - Changed buildGtestParameters() into initGtestParameters() and removed the
> > need for Harness::gtestArgc_ and Harness::gtestArgv_
> >
> > Changes in v5:
> > - Thanks to Niklas:
> > - Moved buildGtestParameters() inside run()
> >
> > No changes in v4
> >
> > src/lc-compliance/main.cpp | 68 +++++++++++++++++++++++++++++++++++---
> > 1 file changed, 64 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/lc-compliance/main.cpp b/src/lc-compliance/main.cpp
> > index 8c18845141de..bf9920008827 100644
> > --- a/src/lc-compliance/main.cpp
> > +++ b/src/lc-compliance/main.cpp
> > @@ -20,6 +20,8 @@ using namespace libcamera;
> >
> > enum {
> > OptCamera = 'c',
> > + OptList = 'l',
> > + OptFilter = 'f',
> > OptHelp = 'h',
> > };
> >
> > @@ -45,13 +47,17 @@ public:
> > ~Harness();
> >
> > int init();
> > - int run(int argc, char **argv);
> > + int run(char *arg0);
> >
> > private:
> > + int initGtestParameters(char *arg0);
> > void listCameras();
> >
> > OptionsParser::Options options_;
> > std::unique_ptr<CameraManager> cm_;
> > +
> > + const std::map<std::string, std::string> gtestFlag_ = { { "list", "--gtest_list_tests" },
> > + { "filter", "--gtest_filter" } };
>
> Flags
>
> > };
> >
> > Harness::Harness(const OptionsParser::Options &options)
> > @@ -76,6 +82,12 @@ int Harness::init()
> > return ret;
> > }
> >
> > + /*
> > + * No need to initialize the camera if we'll just list tests
> > + */
>
> Fits on one line
>
> > + if (options_.isSet(OptList))
> > + return 0;
> > +
> > if (!options_.isSet(OptCamera)) {
> > std::cout << "No camera specified, available cameras:" << std::endl;
> > listCameras();
> > @@ -97,9 +109,11 @@ int Harness::init()
> > return 0;
> > }
> >
> > -int Harness::run(int argc, char **argv)
> > +int Harness::run(char *arg0)
> > {
> > - ::testing::InitGoogleTest(&argc, argv);
> > + int ret = initGtestParameters(arg0);
> > + if (ret)
> > + return ret;
> >
> > testing::UnitTest::GetInstance()->listeners().Append(new ThrowListener);
> >
> > @@ -112,12 +126,58 @@ void Harness::listCameras()
> > std::cout << "- " << cam.get()->id() << std::endl;
> > }
> >
> > +int Harness::initGtestParameters(char *arg0)
> > +{
> > + int argc = 0;
>
> Empty line
>
> > + /*
> > + * +2 to have space for both the 0th argument that is needed but not
> > + * used and the null at the end.
> > + */
> > + char **argv = new char *[(gtestFlag_.size() + 2)];
> > + std::string filterParam;
> > +
> > + if (!argv)
> > + return -ENOMEM;
> > +
> > + argv[argc] = arg0;
>
> argv[0]
This and all of the above fixed.
>
> Does ::testing::InitGoogleTest wants as argv[0] the main function
> argv[0] ? I've no reasons to think it doesn't but I don't get why :)
Well, it's not that gtest wants argv[0] to do something useful, but it does
expect something there. I've tried skipping setting argv[0], but then gtest
crashes, since its parameter parsing routine probably expects a valid char
pointer there. But I don't think the string really matters.
So it was either passing argv[0] from main to it or creating an empty string to
pass it, and I thought the former would be less confusing even though it's more
work for something not useful (at least not to my knowledge). I could certainly
do the latter instead if you think that's better.
And at the time I didn't realize InitGoogleTest() copied the parameters
internally, so I thought we'd need to keep the empty string on the Harness class
scope. But since it does we could just create an empty string local to this
function and use that as arg0, which is way better.
>
>
> > + argc++;
> > +
> > + if (options_.isSet(OptList)) {
> > + argv[argc] = const_cast<char *>(gtestFlag_.at("list").c_str());
> > + argc++;
> > + }
> > +
> > + if (options_.isSet(OptFilter)) {
> > + /*
> > + * The filter flag needs to be passed as a single parameter, in
> > + * the format --gtest_filter=filterStr
> > + */
> > + filterParam = gtestFlag_.at("filter") + "=" +
> > + static_cast<const std::string &>(options_[OptFilter]);
> > +
> > + argv[argc] = const_cast<char *>(filterParam.c_str());
> > + argc++;
> > + }
> > +
> > + argv[argc] = 0;
> > +
> > + ::testing::InitGoogleTest(&argc, argv);
> > +
> > + delete argv;
>
> I assume InitGoogleTest copies its arguments in then ?
Yep.
>
> The patch looks good, my only concern is that for any gtest option we
> have to support we'll have to do the converion here. Do you know how
> many options does gtest provides and how many of them could be useful
> ?
These are Gtest's options (from its 'help'):
Test Selection:
--gtest_list_tests
List the names of all tests instead of running them. The name of
TEST(Foo, Bar) is "Foo.Bar".
--gtest_filter=POSITIVE_PATTERNS[-NEGATIVE_PATTERNS]
Run only the tests whose name matches one of the positive patterns but
none of the negative patterns. '?' matches any single character; '*'
matches any substring; ':' separates two patterns.
--gtest_also_run_disabled_tests
Run all disabled tests too.
Test Execution:
--gtest_repeat=[COUNT]
Run the tests repeatedly; use a negative count to repeat forever.
--gtest_shuffle
Randomize tests' orders on every iteration.
--gtest_random_seed=[NUMBER]
Random number seed to use for shuffling test orders (between 1 and
99999, or 0 to use a seed based on the current time).
Test Output:
--gtest_color=(yes|no|auto)
Enable/disable colored output. The default is auto.
--gtest_brief=1
Only print test failures.
--gtest_print_time=0
Don't print the elapsed time of each test.
--gtest_output=(json|xml)[:DIRECTORY_PATH/|:FILE_PATH]
Generate a JSON or XML report in the given directory or with the given
file name. FILE_PATH defaults to test_detail.xml.
--gtest_stream_result_to=HOST:PORT
Stream test results to the given server.
Assertion Behavior:
--gtest_death_test_style=(fast|threadsafe)
Set the default death test style.
--gtest_break_on_failure
Turn assertion failures into debugger break-points.
--gtest_throw_on_failure
Turn assertion failures into C++ exceptions for use by an external
test framework.
--gtest_catch_exceptions=0
Do not report exceptions as test failures. Instead, allow them
to crash the program or throw a pop-up (on Windows).
Except for --gtest_list_tests, you can alternatively set the corresponding
environment variable of a flag (all letters in upper-case). For example, to
disable colored text output, you can either specify --gtest_color=no or set
the GTEST_COLOR environment variable to no.
There are quite a few, but most don't seem that useful to me personally.
Thanks,
Nícolas
>
> Thanks
> j
>
> > +
> > + return 0;
> > +}
> > +
> > static int parseOptions(int argc, char **argv, OptionsParser::Options *options)
> > {
> > OptionsParser parser;
> > parser.addOption(OptCamera, OptionString,
> > "Specify which camera to operate on, by id", "camera",
> > ArgumentRequired, "camera");
> > + parser.addOption(OptList, OptionNone, "List all tests and exit", "list");
> > + parser.addOption(OptFilter, OptionString,
> > + "Specify which tests to run", "filter",
> > + ArgumentRequired, "filter");
> > parser.addOption(OptHelp, OptionNone, "Display this help message",
> > "help");
> >
> > @@ -147,5 +207,5 @@ int main(int argc, char **argv)
> > if (ret)
> > return ret;
> >
> > - return harness.run(argc, argv);
> > + return harness.run(argv[0]);
> > }
> > --
> > 2.32.0
> >
More information about the libcamera-devel
mailing list