googletest icon indicating copy to clipboard operation
googletest copied to clipboard

Consider calling `absl::ParseCommandLine` from `gtest_main.cc`

Open cramertj opened this issue 3 years ago • 8 comments

Since gtest_main intercepts argc and argv, users are not given the opportunity to parse flags unless they write a custom main function. Consider calling absl::ParseCommandLine from gtest_main (or a similar ABSL-specific testing main file) so that users aren't confused why their command line flags aren't being parsed, and so that they don't then have to write their own custom main targets.

cramertj avatar Nov 03 '21 23:11 cramertj

@cramertj Could you please elaborate on the following:

users aren't confused why their command line flags aren't being parsed

When exactly does that happen?

they don't then have to write their own custom main targets

Why would somebody want to write tests without writing a custom main target?

joshiayush avatar Jan 01 '22 02:01 joshiayush

When exactly does that happen?

See https://github.com/tensorflow/federated/pull/2104 for an example. Essentially, when bringing @com_google_googletest//:gtest_main into your test target to provide a main function while also using ABSL_FLAG. Since @com_google_googletest//:gtest_main does not parse the command line arguments, no flags are available to the tests.

Why would somebody want to write tests without writing a custom main target?

This is standard practice when using this library inside google. The main target is just a standard stub which calls RUN_ALL_TESTS, a la @com_google_googletest//:gtest_main or my own modification to it which parses command line arguments: https://github.com/tensorflow/federated/pull/2104/files#diff-4878c576f517ccb0969f3e10874df54cb99903b415ce4773fffdb2a5cfd434c7

Most tests don't need to write a custom main function.

cramertj avatar Jan 04 '22 17:01 cramertj

@derekmauro Are there any plans to include what @cramertj says?

Consider calling absl::ParseCommandLine from gtest_main (or a similar ABSL-specific testing main file)

This seems to be a good enhancement, I mean the workaround is still good you don't have to do a bunch of work there but it's a good idea to separate ABSL-specific test targets.

joshiayush avatar Jan 05 '22 06:01 joshiayush

I'm working on a change to use the Abseil flags library in GoogleTest when built with --define=absl=1. This should address this issue.

derekmauro avatar Mar 08 '22 16:03 derekmauro

I just pushed a change to use Abseil flags in GTest. Please give it a try.

derekmauro avatar Apr 04 '22 14:04 derekmauro

Why would somebody want to write tests without writing a custom main target?

I am trying to test a regular expression (RE2) parser which uses abseil flags. The flags are RE2 expressions. I use the args capability of bazel to specify possible RE command line flags. I would prefer one test.cc file and I would like the different bazel test targets to specify different args to test the parser and the args to subset what tests in test.cc are run --gtest_filter capability.

As it is now, absl::ParseCommandLine() fails on --gtest_filter=xxxx.yyy.

Perhaps I can use the environment variable to subset.

nsdjg avatar Jun 03 '22 20:06 nsdjg

Why would somebody want to write tests without writing a custom main target?

I am trying to test a regular expression (RE2) parser which uses abseil flags. The flags are RE2 expressions. I use the args capability of bazel to specify possible RE command line flags. I would prefer one test.cc file and I would like the different bazel test targets to specify different args to test the parser and the args to subset what tests in test.cc are run --gtest_filter capability.

As it is now, absl::ParseCommandLine() fails on --gtest_filter=xxxx.yyy.

Perhaps I can use the environment variable to subset.

Yes, using GTEST_FILTER works.

nsdjg avatar Jun 03 '22 20:06 nsdjg

I'm working on a change to use the Abseil flags library in GoogleTest when built with --define=absl=1. This should address this issue.

It works! Thank you!

I think this issue can be closed.

luangong avatar Jan 17 '24 12:01 luangong