Switch unit tests to Catch2 framework

[ferdnyc]

This PR moves us away from the UnitTest++ framework (which has been in a state of suspended development for some time now), replacing it with Google's Catch2 framework.

In the process of making this change, it also:

1. Switches to using CTest to manage the test runs
2. Renames the test sources from tests/FooClass_Tests.cpp to tests/FooClass.cpp
3. Builds a separate test executable openshot-FooClass-test for each source file. (Yay! No more test cases stomping all over each other and messing up the results.)
4. Alters how coverage stats are collected, when enabled
5. Alters how coverage is set up, when not enabled (A coverage target is now always created, and can be used to execute the unit tests. If coverage is enabled, it'll be collected. Building the coverage target is now the preferred way of running unit tests in all cases, for all versions of CMake.)
6. Preserves a copy of the old tests files in tests/cppunittest/, for temporary fallback

So, the changes in the PR are fairly significant and sweeping, even though they affect very little other than the unit tests.

Fallback to UnitTest++ test files

Is removed.

A new CMake option USE_CATCH2 is ON by default, but for now it can be turned off if necessary and CMake will fall back to the old tests. (So, run cmake -DUSE_CATCH2=0 … to use UnitTest++ tests.) This is a very short-term temporary measure, just until all of our build systems are configured with Catch2. We'd really rather not maintain two versions of the tests in parallel, so the old UnitTest++ sources should be considered frozen and subject to imminent removal.

Parallel CTest runs by default

Because CTest runs each unit test as a separate process (the test runner for the test is executed once for each test, and performs only one test per run), it can run a bit slower than the single-binary UnitTest++ execution. To help offset that, the Catch2-based (aka CTest-based) unit tests will be configured to run in parallel by default, using however many processors are detected on the system. This can be disabled by setting -DENABLE_PARALLEL_CTEST=0 on the cmake command line.

Writing tests for Catch2

From the perspective of writing unit tests, Catch2 is very similar to UnitTest++. Most UnitTest++ macros have a direct mapping into equivalent Catch2 syntax, and only minor adjustments to syntax are required to switch from one to the other. In fact, the conversions in this PR were largely done by regular expression replacements run over the entire tests/ directory, plus some manual cleanup to fix cases my inital expressions didn't properly account for.

What follows is a rough guide to how our old UnitTest++ code can be / has been translated into Catch2 code. It covers only the UnitTest++ features we actually made use of. Catch2 also offers additional functionality beyond what's required to provide a subset of UnitTest++'s features, and anyone adding to our test codebase should feel free to make use of any of those features in structuring new or existing tests. See the Catch2 documentation for complete details.

Converting UnitTest++ code into Catch2 code

TEST() becomes TEST_CASE(), with significantly different syntax

Calls to the TEST() macro from UnitTest++ can generally be replaced with TEST_CASE() calls in Catch2. However, the syntax is quite different between the two, more than any other change here.

• In UnitTest++, TEST() takes a class/function name as test identifier, e.g. TEST(Default_Constructor). If the names are wrapped in a SUITE() identifier, they need not be unique between files, only within a given file.

• In Catch2, the TEST_CASE() macro accepts test names as free-form strings, which must be double-quoted but can contain spaces and punctuation. There is no SUITE() macro, however because we are building separate executables for each source file, the names need not be unique between files.
  TEST_CASE() also accepts a second argument, a string containing a list of tags applied to each test. Each tag is enclosed in square braces, and any number of tags can be applied by concatenating them. I have tagged each of the existing tests with [libopenshot] and the tested class name (e.g. [frame] or [imagereader]), and have additionally tagged [opencv] on any tests involving the CV code.

Example:

// Before, in tests/FrameMapper_Tests.cpp
SUITE(FrameMapper) {
TEST(Default_Constructor)
{
  …
}
}

// After, in tests/FrameMapper.cpp
TEST_CASE("default constructor", "[libopenshot][framemapper]")
{
  …
}

(However, because the macros were modified by regular expressions, most of them are currently of the form TEST_CASE("Default_Constructor", …) — this should not be interpreted as a canonical or preferred formatting, and anyone should feel free to write new TEST_CASE()s in Catch2 style, as well as to submit PRs updating the names for existing tests should they feel so inclined.)

For the most part, the only test macro needed is: CHECK(), which takes a single argument.

(Catch2 also supports REQUIRE(), which unlike CHECK() will terminate the TEST_CASE() on failure.)

• UnitTest++ had CHECK(), CHECK_EQUAL(), and CHECK_CLOSE(), which took one, two, and three arguments respectively.

• Catch2 replaces all of those with single-argument macros, which take simple boolean comparisons: a == 4, p != nullptr, pi < 3.15, etc. Nothing more complex — no boolean algebra, nothing involving && or ||. The test must be a single boolean operation.

Examples:

// Before
CHECK_EQUAL(true, r.info.has_video);
CHECK_EQUAL(1, f.number);
CHECK_EQUAL(openshot::InterpolationType::BEZIER, p.interpolation);
// After
CHECK(r.info.has_video == true);
CHECK(f.number == 1);
CHECK(p.interpolation == openshot::InterpolationType::BEZIER);

Pretty straightforward.

A negated form, CHECK_FALSE(), can optionally be used to handle certain specific cases

The only other macro used for comparisons is CHECK_FALSE() / REQUIRE_FALSE(), necessary because CHECK() is incompatible with logically negated expressions. So:

// instead of
CHECK(!haz_cheezburger);  // fails
// you can write
CHECK_FALSE(haz_cheezburger);  // passes

The above could also be written CHECK(haz_cheezburger == false), tho, so you never have to use CHECK_FALSE(). Still, it's there if you prefer. Either way, the important thing to bear in mind is that CHECK(!something); won't work.

Instead of CHECK_CLOSE(), use the Approx() helper in CHECK() comparisons on floating-point values

To handle imprecise floating-point checks, Catch2 provides an Approx() helper class that can be used to perform fuzzy comparisons with float values. Approx() offers three methods for setting the required precision for the comparison, the most directly equivalent to CHECK_CLOSE() being Approx().margin().

Example:

// Before
CHECK_CLOSE(29.97, r->info.fps, 0.01);
// After
CHECK(r->info.fps == Approx(29.97).margin(0.01));

See the documentation for details on the other two methods, .epsilon() and .scale().

CHECK_THROW() becomes CHECK_THROWS_AS()

• UnitTest++ tests for exceptions using CHECK_THROW(expression, exception_class).
• Catch2 has a CHECK_THROW(expression), which only checks that the expression throws any exception. Generally you should instead use CHECK_THROWS_AS(), which takes the same two arguments as UnitTest++'s CHECK_THROW() and offers identical functionality.
• (Don't overlook the change in tense: that's THROWS, with an "S".)

[codecov]

Codecov Report

Merging #635 (2255852) into develop (982558e) will increase coverage by 0.32%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop     #635      +/-   ##
===========================================
+ Coverage    52.10%   52.42%   +0.32%     
===========================================
  Files          151      151              
  Lines        12332    12346      +14     
===========================================
+ Hits          6426     6473      +47     
+ Misses        5906     5873      -33     

Impacted Files	Coverage Δ
tests/CVStabilizer.cpp	100.00% <100.00%> (ø)
tests/CVTracker.cpp	100.00% <100.00%> (ø)
tests/CacheDisk.cpp	100.00% <100.00%> (ø)
tests/CacheMemory.cpp	100.00% <100.00%> (ø)
tests/Clip.cpp	100.00% <100.00%> (ø)
tests/Color.cpp	100.00% <100.00%> (ø)
tests/Coordinate.cpp	100.00% <100.00%> (ø)
tests/DummyReader.cpp	100.00% <100.00%> (ø)
tests/FFmpegReader.cpp	100.00% <100.00%> (ø)
tests/FFmpegWriter.cpp	100.00% <100.00%> (ø)
... and 22 more

---

Continue to review full report at Codecov.

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

[ferdnyc]

Added a commit to prevent CacheDisk temporary directories from trampling all over each other when tests are run in parallel, and another to fix some minor issues Codacy flagged.... and I think this is probably good to go. I already tested it seven ways from Sunday before opening the PR, so I feel it's pretty solid. Still, I'll let it stew for a couple of days at least in case anyone wants a looksee. (@jonoomph?)

[github-actions]

Merge conflicts have been detected on this PR, please resolve.

[github-actions]

Merge conflicts have been detected on this PR, please resolve.

[ferdnyc]

I'mma aim towards merging this soon, hopefully today -- I just need to go through one more time and make sure that any recent changes to the UnitTest++ tests are synced up to the Catch2 versions.

I was hoping to get them working on macOS and Windows as well, but that'll have to be a future PR if it happens at all. Trying to maintain a PR with "alternative" unit tests that every test edit has to be manually synced with is too much hassle, and the Catch2 tests have already proven themselves just as effective. Switching over to writing them shouldn't require much more than a brief adjustment period to get used to the new syntax. (See my guide above.)

So, last chance to have a looksee... @jonoomph? Anybody?

[github-actions]

Merge conflicts have been detected on this PR, please resolve.

[ferdnyc]

I replaced the branch history with a cleaner version, and removed the UnitTest++ fallback. That's primarily so that any open PRs or new commits that make changes to the old tests will now conflict with this code after merge, instead of sneaking changes into the fallback dir that aren't ported to the new ones.

[github-actions]

Merge conflicts have been detected on this PR, please resolve.

[ferdnyc]

As this is once again synced up with the current develop code, and even contains some improvements (some memory leaks fixed), I'm going to merge it. I don't believe there are many PRs open right now that make changes to unit tests, so it shouldn't create too many conflicts hopefully.

I'm not at all sure that everyone else who might be interested has seen these changes and won't be surprised when the unit tests suddenly change completely, but everyone has had weeks to look at this and it's not like I can force anyone to read a PR.

[jonoomph]

@ferdnyc This all looks great! Sorry for the delays on my side (feedback wise). What version of Catch2 should I install on our builders? Do you have any suggestions, regarding build instructions, or specific versions to target?

[jonoomph]

Okay, I used the *.deb package you linked to on our Ubuntu builder, that worked great. I installed catch2 using brew on Mac, and that seems to have worked as well. Last up... Windows (if you haven't done that yet).

[ferdnyc]

@jonoomph windows is all set already, yes. With Catch2, anyway. The tests don't actually WORK if you try to run them, because all of the required DLLs aren't in place. So I'll have to figure out some PATH and/or file-copying shenanigans if we're going to actually use them

[jonoomph]

@ferdnyc Are the catch2 tests optional in the develop branch? I just want to make sure we don't have lots of people frustrated with a broken build, if they are currently missing catch2. Sorry, not at my computer currently or I would just look, lol.

[ferdnyc]

@jonoomph They are now, yeah, I'd screwed that up at first, but fixed it with #656 .

[ferdnyc]

@jonoomph

On the Windows tests, somebody on the CMake forum was discussing methods of dogfooding during a build, and mentioned that the first "unit test" in their test setup is to run a cmake --install into the unit test build directory. That way, the tests have access to the just-built output files. I'm playing with that now, looks like it should work.

Ideally you'd want to build the tests against those test-installed files, in that scenario, but that'd be trickier since the cmake --install would have to be inserted into the middle of the build process, rather than making it a unit test. ...Actually, wait, no. It'd be trickier than that. You couldn't even configure the tests until after the install. Bah, that's too complicated, screw it. Installing at the start of testing (and only on Windows) is good enough.

(And sadly, all this still doesn't solve that annoying "if you try to run the tests before building the project, the tests just fail" weirdness. I just tried, and it seems cmake --install doesn't force a build any more than make test and/or ctest does. The install just fails the same way.)

[ferdnyc]

Hah! "Worked" (on Linux), but the install ended up being the LAST test run instead of the first, which is less than useful. But as soon as I figure out ordering in CTest, should do the trick. Though there's still libopenshot-audio to consider, if it was only installed to a build directory and isn't available in any of the system default locations.

[ferdnyc]

Oh, fuck me!

I really thought I had it working, even figured out a trick for copying libopenshot-audio.dll as well. And it does indeed make the unit test executables runnable from the <builddir>/tests/ directory.

BUT! Because of the way the CMake Catch module's self-discovery feature works, the list of tests is constructed by running each Catch-enabled unit test executable with a -l argument (to list the set of tests it contains, which are then each added to CTest). That's done right after building the executable, BEFORE the DLLs are copied. Which means that when it tries to discover tests, all the executables fail to run, and the tree ends up with no tests defined other than my DLL-install "test"!

So the library copy really does have to happen before the tests are built. FNARD.

cc: @jonoomph

[ferdnyc]

I'll have to set this aside for tonight, and hopefully figure it out tomorrow.

[ferdnyc]

WOO! Got it, unit tests are working under Windows. Will submit as part of my mingw-paths branch PR.

(And in running them, I discovered that the .webm encoder was broken due to an ABI mismatch in the ffmpeg packages -- see msys2/MINGW-packages#8404 -- so I downgraded libvpx to the previous, compatible version in both 32- and 64-bit MinGW.)

[ferdnyc]

And with a quick rebuild of the mingw-w64-*-ffmpeg packages by the MSYS team, libvpx-1.10.0 is again working for de/encoding vp8 and vp9 video; I've re-upgraded the Windows builders with libvpx 1.10.0 and the compatible new ffmpeg 4.3.2-2 build.