Add tests for sf::String

[ChrisThrasher]

Description

These tests don't bother covering all edge or corner case behavior. Their intent is to test all the simple cases of sf::String and lay the groundwork for more sophisticated tests which may come later. I'm not experienced with wide strings or UTF-32 strings so I'm not even sure how to tackle testing the more complicated uses.

[Bromeon]

Great work, thanks a lot!

I think the tests may benefit from using some actual wide-char or UTF-32 characters where applicable, so we don't just have ASCII. See https://stackoverflow.com/q/6796157 for string literals 🙂

[ChrisThrasher]

I wonder why there's no code coverage comment on this PR

EDIT: It's working now

[codecov]

Codecov Report

Merging #2466 (e9963d3) into master (741fe21) will increase coverage by 1.32%.
The diff coverage is n/a.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2466      +/-   ##
==========================================
+ Coverage   22.72%   24.04%   +1.32%     
==========================================
  Files         226      226              
  Lines       19388    19409      +21     
  Branches     4705     4714       +9     
==========================================
+ Hits         4406     4667     +261     
+ Misses      14469    14240     -229     
+ Partials      513      502      -11     

Impacted Files	Coverage Δ
include/SFML/System/String.hpp	100.00% <ø> (ø)
include/SFML/System/Utf.inl	80.72% <ø> (+44.01%)	⬆️
src/SFML/System/String.cpp	99.50% <ø> (+85.00%)	⬆️

... and 2 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 741fe21...e9963d3. Read the comment docs.

[ChrisThrasher]

Oh joy. It appears that many of the tests fail on Windows. Is this a bug in my tests or is this simply the reality of the implementation-defined behavior of wide strings? Is it a locale issue? If we're dealing with implementation-defined behavior then I'm not sure whether we can test most of these things to any meaningful degree.

[ChrisThrasher]

I think I found a satisfactory way to handle the fact that wide string encoding different on a per-platform basis. I'm using the width of a wchar_t as a proxy for which encoding its using. Then I have this function that lets us specify both potential return values for a function and the correct one will get returned.

 // Return either argument depending on whether wchar_t is 16 or 32 bits
// Lets us write tests that work on both Windows where wchar_t is 16 bits
// and elsewhere where it is 32. Otherwise the tests would only work on
// one OS or the other.
template <typename T>
T select(const T& t16, const T& t32)
{
    assert(t16 != t32);
    if constexpr (sizeof(wchar_t) == 2)
        return t16;
    else
        return t32;
}

A better function name probably exists but I kept it simple to begin with.

[ChrisThrasher]

I had to make sf::String::InvalidPos not constexpr to fix some MinGW linker errors. Not sure why MinGW doesn't agree with every other implementation but it's a small price to pay. Reminds me of the woes we had with the sf::Vector constants that we couldn't make constexpr.

[ChrisThrasher]

I think it's all good now

[eXpl0it3r]

Some minor questions and I think we should "document" getSize when clearing, erasing or adding.

[ChrisThrasher]

I believe I addressed all the comments

[ChrisThrasher]

I added more comments to clarify what's going on with these doctest::StringMaker specializations