Permalink
Browse files

Escape quotes, newlines, and similar when printing C strings.

The implementation uses std::string which may violate the requirement that CxxTest should be able to run w/o a standard C library.

It also leaks the buffer returned. I don’t consider this crucial as unit tests are short-lived and run in their own process.
  • Loading branch information...
1 parent 7060d77 commit f50d7d67a41ba76c6c2db2e41e1619e0b99beae6 @sorbits sorbits committed Apr 13, 2010
Showing with 17 additions and 1 deletion.
  1. +17 −1 cxxtest/ValueTraits.h
View
@@ -197,7 +197,23 @@ class ValueTraits<const char * const &> {
public:
ValueTraits(const char * const &value) : _asString(value) {}
ValueTraits(const ValueTraits &other) : _asString(other._asString) {}
- const char *asString(void) const { return _asString; }
+ const char *asString( void ) const {
+ std::string tmp(1, '"');
+ for (char const* src = _asString; src && *src; ++src) {
+ switch(*src) {
+ case '\\': tmp += "\\\\"; break;
+ case '\n': tmp += "\\n"; break;
+ case '\r': tmp += "\\r"; break;
+ case '\t': tmp += "\\t"; break;
+ case '"': tmp += "\\\""; break;
+ default: tmp += *src; break;
+ }
+ }
+ tmp += '"';
+
+ char* res = new char[tmp.size()+1];
+ return strcpy(res, tmp.c_str());
+ }
};
CXXTEST_COPY_TRAITS(const char *, const char * const &);

4 comments on commit f50d7d6

Owner

whart222 replied Mar 3, 2012

I just backed out this change. It was causing tests to fail.

What motivated the need for this revision?

Contributor

sorbits replied Mar 4, 2012

The \t, \r, and \n characters are all “ invisible” so if e.g. testing a function to harmonize line endings or indentation, the test output without using the escaped form will not be helpful.

By doing this, escaping backslash naturally follows to keep it from being ambiguous.

Escaping of the double-quote was added for aesthetic reasons (now that escapes were introduced). The entire string is double-quoted in the error output so it seemed wrong to have non-escaped double-quotes inside the string.

As for breaking tests, I assume it is breaking tests about how asString should format a string? I.e. the patch should have updated the test as well for the new output.

Owner

whart222 replied Mar 4, 2012

Contributor

sorbits replied Mar 7, 2012

The motivation for making them visible would be when testing code like the sample below:

std::string tabs_to_spaces (std::string const& src, size_t tabSize)
{
    return src;
}

std::string unix_linefeeds (std::string const& src)
{
    return src;
}

class InvisibleCharTests : public CxxTest::TestSuite
{
public:
    void test_invisible_char ()
    {
        TS_ASSERT_EQUALS(tabs_to_spaces("\ttest\n  \ttest\n", 4), "    test\n    test\n");
        TS_ASSERT_EQUALS(unix_linefeeds("test\r\ntest\r\n"),      "test\ntest\n");
    }
};

Without escaping at least \t and \r, the error output from the above failing tests would look like the correct output was produced (depending on terminal tab size and how it interprets \r).

The \n is less important to escape, but for example we could do a code indenter where we test how it handles lines with just whitespace. The amount of whitespace on these would also be invisible if \n is rendered as an actual newline.

Another reason to escape \n is to ensure the error output only takes up a single line — though the importance of this depends on build environment and formatter, e.g. I use the unix output and Makefiles with lots of parallel jobs, so if multiple build jobs output multi-line errors at the same time, I see intermixed output, but this is more a failure of make.

Please sign in to comment.