boostorg / core

Boost Core Utilities
132 stars 86 forks source link

BOOST_TEST_EQ does not print C strings #91

Open Lastique opened 3 years ago

Lastique commented 3 years ago

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 commented 3 years ago

There's also a similar overload for char*.

pdimov commented 3 years ago

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.

Lastique commented 3 years ago

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.

pdimov commented 3 years ago

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?

Lastique commented 3 years ago

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 commented 3 years ago

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

pdimov commented 3 years ago

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.

glenfe commented 3 years ago

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.