core icon indicating copy to clipboard operation
core copied to clipboard

BOOST_TEST_EQ does not print C strings

Open Lastique opened this issue 4 years ago • 8 comments

BOOST_TEST_EQ that involve C-style strings do not output string contents. For example, BOOST_TEST_EQ(*++itr, "foo"); shows this on failure:

libs\filesystem\test\path_test.cpp(341): test '*++itr == "foo"' ('"c:"' == '00007FF79AE87414') failed in function 'void __cdecl `anonymous-namespace'::iterator_tests(void)'

This seems to be caused by this test_output_impl overload:

https://github.com/boostorg/core/blob/e53393357f32078f55cdb291a536bb567895bc62/include/boost/core/lightweight_test.hpp#L167

Is this intended to return const void* instead of const char*?

If yes, then could we make this behavior optional?

Lastique avatar Jun 03 '21 21:06 Lastique

There's also a similar overload for char*.

Lastique avatar Jun 03 '21 21:06 Lastique

We have a separate macro for C strings, which is BOOST_TEST_CSTR_EQ. BOOST_TEST_EQ tests for equality using ==, which for char* compares addresses, so the output also prints addresses to match that. BOOST_TEST_CSTR_EQ test for equality with strcmp, and prints the C strings to match.

pdimov avatar Jun 03 '21 22:06 pdimov

BOOST_TEST_EQ tests for equality using ==, which for char* compares addresses, so the output also prints addresses to match that.

In Boost.Filesystem case we are comparing path with string literals using operator==, which is intended. strcmp wouldn't work there.

Lastique avatar Jun 03 '21 23:06 Lastique

Yes, I understand that, but how are we supposed to distinguish the two cases? In general we have no idea what == does. We could special-case std::string, I suppose, and then it won't work for string_view, so we'll have to special case that too?

pdimov avatar Jun 03 '21 23:06 pdimov

I think, the special case is comparing two pointers and the rest can be assumed to operate as intended (i.e. there must be a user-defined comparison or conversion operator for the comparison to compile).

Lastique avatar Jun 04 '21 00:06 Lastique

Also, the comparison logic is unrelated to output formatting, which is what this issue about.

Lastique avatar Jun 04 '21 00:06 Lastique

Not entirely unrelated; when the test fails the values need to tell us why.

the special case is comparing two pointers and the rest can be assumed to operate as intended

This might be a good heuristic, yes (we also need to take arrays into account). Still a bit dangerous for my taste because misinterpreting a char* that doesn't point to a C string would be bad... but it might be acceptable if we take care to quote the unprintable characters, which is a good thing to have anyway and something I've been thinking we need to do.

pdimov avatar Jun 04 '21 00:06 pdimov

I'd prefer not assuming (by default) that char* means C string. If someone wants to test some operator==(const X&, const char*) with BOOST_TEST_EQ(x, "y") then maybe we could provide some alternative macro that allows supplying a mechanism to control how the LHS and RHS arguments are formatted.

glenfe avatar Jun 04 '21 01:06 glenfe