--run_test in Boost 1.62 does not accept test names which contain ':'

[Igor Akhmetov <igor.akhmetov@…>]

Because ':' gets interpreted as a filter separator.

Names that contain a colon are pretty common - one way to get one is to make a test parameterized with a type which is a namespace member:

namespace ns { struct X {}; }
typedef boost::mpl::list<ns::X> test_types;
BOOST_AUTO_TEST_CASE_TEMPLATE(test, T, test_types) {}

[Raffi Enficiaud]

Hi,

Do you have any other use case in mind? We may think of an escaping of the relevant ":" like "ns\:\:X" for that purpose. Would that do?

[igor.akhmetov@…]

Hello,

No, I just want a reliable way to run a specific test (from a test runner).

Escaping colons would work, except for manually registered tests with names that contain "\:" - but it's probably a very rare case and not worth being worked around. But I still find it unfortunate that backwards compatibility was broken with Boost.Test v3.3, since previously valid command lines stopped working.

Thanks!

[Raffi Enficiaud]

Bugs are always unfortunate.

For the story, this colon was introduced to address the issue #11859 in order to be able to specify several test cases from the associated environment variable. There is not so many chars left in fact, as we should not interfere with the terminal command line parsing (& ; * % $ | are not possible choices, @ ! + - / are already in use).

At that time, : seemed to me the best choice as I (wrongly) thought we cannot generate test names with :.

Apart from escaping or protecting with surrounding "", I do not see any good solution. If surrounding with "" would do for you, let's just go for that.

[Igor Akhmetov <igor.akhmetov@…>]

Sorry, I don't quite understand how surrounding with "" would help. Quoting command line arguments is already required though to correctly pass test names which contain quotes or spaces.

Is there actually any advantage to adding a filter separator? Since Boost.Test accepts multiple -t arguments, the only use case seems to be to specify several filters from the environment variable. But (specifically on Windows) the maximum length of a value of an environment variable and maximum command line length are both 32K, so from the POV of a test runner, both ways of specifying a test filter have similar limits. It might be useful on POSIX platforms though.

[Raffi Enficiaud]

Quoting command line arguments is already required though to correctly pass test names which > contain quotes or spaces.

I am not sure to understand: do you have test names with quotes or spaces?

Quoting would help in the sense that ':' would not be interpreted as a filter separator within quotes, which in turn would remove the need for back-slashing the ':'. For your own use case, it would use at much as many additional chars, but would have better readability ("ns::X" and ns\:\:X have both 2 additional chars, but "ns::X::Y" is more readable and has less chars than ns\:\:X\:\:Y).

You would then have something like

--run_test="ns::X":"ns::Y":suite1/suite2/"ns2::X","ns2::Y"

There is indeed one use case needing specifying filters from environment variable (and it is on the ticket I mentioned).

If it were only me, I would rather remove the support for any special chars from the test unit names and replace everything by "_" (ns_ _X instead of ns::X), that would things much cleaner.

[Igor Akhmetov <igor.akhmetov@…>]

Looks like spaces actually do get replaced with underscores in boost::unit_test::ut_detail::normalize_test_case_name, but my point still stands for quotes. The problem is that boost::unit_test::make_test_case only sanitizes spaces from test names, so this means that a name can still be a pretty arbitrary string for tests manually added from init_unit_test.

Using quotes like that won't really help, since cmd.exe will eat them away. So e.g. when the test binary is run with

--run_test="ns::X":"ns::Y"

the actual argument in argv will be "--run_test=ns::X:ns::Y". Escaping quotes to get them passed to main would be somewhat ugly and would require more complicated parsing.

Any form of sanitizing would really work, so long as it's possible to take a test name reported by --list_content and pass it to -t. Given that spaces are already replaced, I don't see why replacing colons with underscores is a bad idea?

[Raffi Enficiaud]

Yes, you are right for the quotes, they will be eaten by the cmd line.

I am perfectly fine with sanitizing the test case/suite names better and this is for me the right solution (and having ':' in test names is for me a bug). Underscores (which would expand to double underscore for ::) would be a good solution for me.

Let's go for this then.

[Raffi Enficiaud]

Would you please test the branch topic/12531-run-test-with-colons and let me know if this works for you?

Thanks

[Igor Akhmetov <igor.akhmetov@…>]

Thanks, that works!

Less significant than the original problem, but manually added tests still can be registered with names that can't be passed back to --run_test. Why not sanitize the rest of meta-characters in test names? As far as I can tell, at least ,*@+!/ need to be replaced as well.

Not related to this issue per se, but it would be nice if Boost.Test reported if two test cases have the same sanitized id. Normally they shouldn't, since all data test subcases now have unique identifiers, but if there are two tests with the same id, it's impossible to run only one of them. Should I file a separate issue for that request?

[Raffi Enficiaud]

Thanks for all those suggestions and for testing the branch. I just fixed another issue when file names contain actually a ":" (eg. on Windows in the form "C:
something"), just in case you hit this problem.

I am leaving this issue open:

• for the other tokens, it totally makes sense, but since nobody is bothered by this (so far), I would prefer closing the 1.63 in a safe manner. It would be nice of you if you can open another issue for this :-)
• for the check on name clashes, I would suggest opening another issue so that we can discuss internally about it

Thanks again.

[Igor Akhmetov <igor.akhmetov@…>]

I filed ​https://svn.boost.org/trac/boost/ticket/12596 for the meta-characters issue and ​https://svn.boost.org/trac/boost/ticket/12597 for name clashes, so I guess this issue can be closed in Boost 1.63.

Thanks!

[Raffi Enficiaud]

Thanks for the additional tickets, closing this now as 1.63 is almost out.