Catch2: String comparison is failing

[retokid]

Description

CHECK(copy == date) will fail with following output:

/.../DateTest.cpp:69: FAILED:
  CHECK( copy == date )
 with expansion:
  "11/02/01" == "11/02/01"

Steps to reproduce

const char * date = "11/02/01";
char copy[9];

for (auto i = 0; i<9; i++)
    copy[i] = date[i];

CHECK(copy == date);

• Catch version: 1.10.0
• Operating System: OSX 10.12.6
• Compiler+version: XCode 9.0

[philsquared]

That's because it's comparing the pointer values, not the strings themselves.
Although I agree it's confusing - it's just C++ confusing, and not anything specific to Catch.

It's the same behaviour as if you did:
assert( copy == date );

[horenmar]

I want to say that the error here is stringifying const char* as a string instead of as a pointer, but then stringification for comparing std::string variable with known string would be useless, so it is a bit of damned if you do, damned if you don't.

[philsquared]

IIRC there has a been a suggestion in the past that we should include both the string _and_ the pointer value in the stringification.
That might be the best trade off.

[retokid]

I'm not sure I understand horenmar's comment, but if we are comparing pointers the output should print pointer values, not strings. Furthermore, in that case, the example given in the reference document:

CHECK( str == "string value" );

is extremely misleading and should be removed.

Last but not least, how would I then compare strings ? I'm trying to migrate from BOOST and these are 90% of my tests...

[horenmar]

@retokid You have to understand that expressions inside Catch's assertions behave exactly as they would outside*. What this means is rather simple, if you consider this piece of code

std::string str = "abc";
const char* cstr1 = "abc";
const char* cstr2 = strdup(cstr1);
assert(str == cstr1); // Passes
assert(str == cstr2); // Passes
assert(cstr1 == cstr2); // Fails
assert(strcmp(cstr1, cstr2) == 0); // Passes

The first two asserts pass because std::string specifically has operator== that takes a const char* and assumes it is a C-style string. The third assert fails, because comparing two pointers of any type is done with their address, rather than contents. The fourth assert then compares the two char*s by content, with the assumption that they are valid C-style string.

The problem we have is that stringification is done context-free, which means that when deciding whether to stringify a char* as a pointer or as a C-style string, we cannot take a look on the type it is compared against. Thus, we stringify char* as C-style string, because it is more likely that someone writes this

std::string str = ...;
REQUIRE(str == "Known good output");

rather than this

const char* cstr = "...";
const char* cstr2 = "...";
REQUIRE(cstr == cstr2);

as the second is quite likely to be wrong.

* Some caveat emptor applies in regards to linkage and number literals

[philsquared]

I agree, on principle, that if we're comparing pointers we should print pointers - and usually we do. char* is a special case, though, where we are usually interested in the null-terminated string it's pointing at.
_But_ that introduces confusion like this where you see two identical strings printed with the report that they are different.
Hence my reference to the proposal that both the pointers _and_ the strings be printed. I think that walks the line best.

As for the example you gave, assuming you mean from here: https://github.com/philsquared/Catch/blob/master/docs/assertions.md#natural-expressions, I believe the implication was that str was a std::string, in which case it would work. Thank you for pointing out the ambiguity, I'll get that clarified in the docs.

To compare strings, if you only have char*s you have the same range of options as you would outside a test framework. You could use C's string comparison functions, e.g. strcmp et al. Or you could wrap one of the operands in a std::string and use the operator - e.g.:

CHECK( std::string(copy) == date );

[philsquared]

sorry, my response crossed with @horenmar, as did our coverage - but there's also some difference so I'll let mine stand :-)

[retokid]

OK, in any case thanks to both for the answers, I appreciate the time you take for answering my rather basic questions.

[retokid]

Using standard libraries is not an option for me, I'm writing embedded code that has to run in 32k code 2k ram. This is the reason I only deal with C strings. I temporarily solved the issue with a:
#define CHECKS(a, b) CHECK(std::string(a) == b)
But shouldn't you simply add a macro to the framework to solve the above mentioned ambiguity ?

[martinmoene]

Can't help the following comes to the fore:

• Presentation by Roger Orr and Steve Love, ACCU 2011.
  Some objects are more equal than others - a look at the many meanings of equality, value and identity
  
  
  
  • Slides
  
  
  • Richard Harris Apppendix Slides
  
  
• Article by Roger Orr and Steve Love.
  Some objects are more equal than others: The many meanings of equality, value and identity.

...the variation between "=" and "==" continues to catch people out!

;)

[lightmare]

Nice explanations, but what if we could prevent users from falling into this trap again and again?

char foo[] = "foo";
CHECK(foo == "foo"); // FAILED with expansion "foo" == "foo"

This could be improved by providing comparison overloads for const char (&)[N] to at least handle string literals, but comparing two const char* variables would still yield surprising results of pointer comparison.

However, that's not the only use case. Sometimes it's useful to compare addresses, right?

const char* p1 = generate_noise(1);
const char* p2 = generate_noise(1);
// let's check that it returns pointer to previously generated noise
// when called with the same seed
CHECK(p1 == p2); // you don't want Catch to stringify this, it's random binary data
CHECK((void*)p1 == (void*)p2);

So, given that catch treats const char* as a pointer to null-terminated string when reporting, why not treat it the same way when comparing? That way Catch will favor the more common use case. And if someone really wants to compare pointers, they can cast to void* explicitly.

[philsquared]

@retokid you could also use:

using namespace Catch::Matchers;
//...
CHECK_THAT( copy, Equals( date ) );

I think it's important that what operators mean within a CHECK or REQUIRE is exactly the same as what they mean outside (with some limitations, as @horenmar has hinted at).

I've been looking, again, at this idea of reporting the pointer value as well as the string for char*s, but I think it's too verbose and more confusing than not (which is probably what I concluded last time too).

[retokid]

Thanks Phil for the hint, I was indeed looking at Matchers to understand if they could help.

My issue with this cased stemmed from :

• the fact I am migrating from BOOST_TEST, in BOOST comparing two const char * by == will compare the string contents,
• the documentation of Catch where we have this CHECK(string = "...") example, and my misunderstanding that string was not standing for a C string but a std:string (I'm not familiar with modern C++ libraries).

Since I had the issue, understood and solved it, I came to one conclusion : the idea that what's inside the CHECK macro should behave exactly as outside is very string principle.

With regards to a CHECK failing because to const char * are not equal, and given that:

• it is extremely simple to add the contents of these strings to the output with the streaming MACROS,
• the output is most of the time only relevant when such tests fail,

I come to the conclusion you should print the pointer values, and not their content.

My opinion only, based on my limited understanding, and I still fully respect your choice to do it the other way around.

[retokid]

Thanks Phil for the hint, I was indeed looking at Matchers to understand if they could help.

My issue with this cased stemmed from :

• the fact I am migrating from BOOST_TEST, in BOOST comparing two const char * by == will compare the string contents,
• the documentation of Catch where we have this CHECK(string = "...") example, and my misunderstanding that string was not standing for a C string but a std:string (I'm not familiar with modern C++ libraries).

Since I had the issue, understood and solved it, I came to one conclusion : the idea that what's inside the CHECK macro should behave exactly as outside is very string principle.

With regards to a CHECK failing because to const char * are not equal, and given that:

• it is extremely simple to add the contents of these strings to the output with the streaming MACROS,
• the output is most of the time only relevant when such tests fail,

I come to the conclusion you should print the pointer values, and not their content.

My opinion only, based on my limited understanding, and I still fully respect your choice to do it the other way around.

[lightmare]

@philsquared

I think it's important that what operators mean within a CHECK or REQUIRE is exactly the same as what they mean outside (with some limitations, as @horenmar has hinted at).

Then that should also apply to operands. char* does not always point to a null-terminated string, yet when being reported this type receives special treatment, backed by the argument that it's usually on one side of a string comparison. When pointers are compared, Catch should report only addresses.

[alanconway]

I just started using check in a mixed C/C++ project so comparing possibly-null C-strings is important. I initially tried Check::Matchers::Equals but it chokes on null pointers. I wrote a simple CStringMatcher that solves the problem nicely for me, perhaps it could be included in a future version of check2:

https://github.com/alanconway/qpid-proton/blob/cpp-catch-tests/tests/include/catch_extra.hpp

It works like the built-in Equals but is over-ridden to handle null const char* correctly:

using Catch::Matchers::Equals;
const char *x, *y ...
CHECK_THAT(x, Equals(y)); // pass if x, y are both null or both non-null and string-equal.