catchorg / Catch2

A modern, C++-native, test framework for unit-tests, TDD and BDD - using C++14, C++17 and later (C++11 support is in v2.x branch, and C++03 on the Catch1.x branch)
https://discord.gg/4CWS9zD
Boost Software License 1.0
18.65k stars 3.05k forks source link

Remove final keyword from first party reporters #2887

Open bearbones opened 3 months ago

bearbones commented 3 months ago

Description I don't know why the first party reporters are "not intended to be inherited from," but for my use case, all I want is a ConsoleReporter that doesn't print the test spec filter at the start of each run. To me, the most reasonable option is to inherit from ConsoleReporter and override noMatchingTestCases() and testRunStarting(), but I can't do that when it's final.

I could just edit ConsoleReporter in our fork, but I would prefer to not have to keep a version that's slightly out of sync with the base repo.

Additional context My team's custom main() generates a ridiculously long filter based on a file that lists excluded tests. This is to accommodate legacy tests that were written with Boost.Test macros. We redefined those macros to map to Catch2 along with some extra legwork for keeping track of suites and constructing case names. Right now it floods the console with about 90k characters.

I also need to print out a special message for our meta-test-runner to recognize at the start and end of runs and cases, and I could do that with an event listener, but it seems more elegant to just override those few ConsoleReporter functions.

horenmar commented 3 months ago

You will end up with your own reporter either way, and I would rather have you copy paste ConsoleReporter into CompanyConsoleReporterDoNotSteal, than derive CompanyConsoleReporterDoNotSteal from the built in one.

The underlying reason is simple, I do not want to have to consider whether a refactoring to reporters changes their behaviour if some of the functions are overridden and some are not. As an example, right now the ConsoleReporter only responds to testCaseEnded event, and ignores partial test cases. If I decide to change this in the future, so that it uses the newer testCasePartialEnded (and testCasePartialStarting for resets) for more granular control over the output, I do not want to have to think about whether the users have overridden these already, whether they call the parent's implementation for them, etc.

horenmar commented 3 months ago

Also I am told that I should ask you if you work at Roblox 😃

bearbones commented 2 months ago

Okay, that makes sense.

And to your question, yeah 😊