The cleanup also found out that custom translation for std-derived
exceptions test wasn't running properly, and fixed that.
We cannot enable the warning globally, because the tests contain
some functions that are unused by design -- e.g. when checking
stringification priority of StringMaker vs range fallback and so
on.
Not all reporters use a format that supports this, so TeamCity
and Automake reporters still do not report it. The console
reporter now reports it even on successful runs, where before
it only reported the rng seed in the header, which was showed
either for failed run, or for run with `-s`.
CLoses#2065
This ended up being a surprisingly large refactoring, motivated
by removing a `const_cast` from `Config`'s handling of reporter
streams, forced by previous commit.
This way it makes much more sense from logically-const point
of view, and also means that concrete implementations don't
have to always have a `mutable` keyword on the stream member.
When the added Bazel configuration flag is enabled,
a default JUnit reporter will be added if the XML
envrioment variable is defined.
Fix include paths for generated config header.
Enable Bazel config by default when building with
Bazel.
Co-authored-by: Martin Hořeňovský <martin.horenovsky@gmail.com>
It has become completely vestigial, as it only ever passed-through
the argument down to a different function, and being private
member function, it didn't even introduce a useful compilation
firewall.
This means that the CLI interface now uses the new key-value oriented
reporter spec, the common reporter base creates the colour implementation
based on the reporter-specific configuration, and it also stores the
custom configuration options for each reporter instance.
Closes#339 as it allows per-reporter forcing of ansi colour codes.
The new reporter spec generalizes key-value options that can be
passed to the reporter, looking like this
`reporterName[::key=value]*`. A key can be either Catch2-recognized,
which currently means either `out` or `colour`, or reporter-specific
which is anything prefixed with `X`, e.g. `Xfoo`.
Test case hashing includes tags and class name
As the hasher involves more code now, it was split out into its own file
and it got its own set of tests.
Closes#2304
This includes always compiling the ANSI and None colour
implementations, as they don't need to touch any platform
specific APIs, and removing their respective compile-time
configuration options.
Because the Win32 colour implementation requires Win32-specific
APIs, it is still hidden behind a compile-time toggle,
`CATCH_CONFIG_COLOUR_WIN32` (renamed from `..._COLOUR_WINDOWS`).
The commandline options for colours were also changed. The
option now uses different name, and allows to select between
different implementations, rather than changing whether
the compiled-in colour implementation is used through
"yes/no/default" options.
Forcing it to be engaged explicitly, either via `op<<`, or by
`ColourGuard::engage`, fixes an issue with multiple `ColourGuard`s
being constructed in a single expression. Because the construction
of the `ColourGuard` instances can happen in arbitrary order,
colours would be applied in arbitrary order too. However, a chain
of `op<<`s has strict call orders, fixing this issue.
FatalConditionHandlerGuard is used within RunContext::invokeActiveTestCase().
The intent of this guard is to avoid binary crash without failed test being
reported.
Still in case FatalConditionHandlerGuard destructor being called during stack
unwinding AND finds unexpected top-level filter for SEH unhandled exception,
the binary may still crash. As result of such crash the original exception
details are being hidden.
As the Catch2 provides only `CATCH_CATCH_ANON` macro, with no access to
exception details by design, looks like the best way to handle issue is to:
- state requirements explicitly by `noexcept` specifier
- use `Catch::cerr()` to print out possible issue notification
Signed-off-by: Kochetkov, Yuriy <yuriyx.kochetkov@intel.com>
* POSIX colour impl is now compiled for all platforms.
* Deciding whether a colour impl should be picked is now stream
dependent, and thus incompatible implementations can be removed
immediately, rather than checking when the colour is being used.
This fixes an issue where reporter with default-output to stdout
would think that it was given a stream _not_ backed by console,
thus not using colour.
This also required splitting out Listener factory from
the reporter factory hierarchy. In return, the listener
factories only need to take in `IConfig const*`, which
opens up further refactorings down the road in the colour
selection and implementation.
By default, CMake derives a Visual Studio project GUID from the
file path but the GUID can be overridden via a property
(see https://gitlab.kitware.com/cmake/cmake/-/commit/c85367f4).
Using a non-constant GUID can cause problems if other projects/repos
want to reference the catch2 vcxproj files, so we force a constant GUID here.
Signed-off-by: Alan Jowett <alanjo@microsoft.com>
Resolves: #2388
This opens path to per-reporter colour output customization,
and fixes multiple issues with the old colour implementation.
Under the old implementation, using Win32-backed colouring
would always change the colour used by the console, even if the
actual output was written elsewhere, such as a file passed by
the `--out` flag. This will no longer happen, as the reporter's
colour impl will check that the reporter's stream is pointed
to console before trying to change the colours.
POSIX/ANSI colour implementation suffered a similar-ish issue,
in that it only wrote the colour escape codes into the default
output stream, even if the reporter asking for colouring was
actually writing to a completely different output stream.
This will become useful when reworking colour support, because
Win32 colour support requires checking whether the output is
stdout, which is done through the `IStream` wrapper.
The cached handle would become invalid if some other code, say
a user-provided test code, redirects stdout through `freopen`
or `_dup2`, which would then cause AppVerifier to complain.
Fixes#2345
At one point it was inserted there as the simplest way to smuggle
around an extra return value for specific errors in executing
tests. Since then, the error has been changed to be handled
differently, and the member became unused.
Turns out people are bad at not combining code compiled with GCC
and Clang, and the improvement from `trivial_abi` on `unique_ptr`
is not worth the maintenance cost of having this be an opt-in
change.
Closes#2344
This avoids issues with Catch2's handler firing too early, on
structured exceptions that would be handled later. This issue
meant that the old attempts at structured exception handling
were incompatible with Windows's ASan, because it throws
continuable `C0000005` exception, which it then handles.
With the new handling, Catch2 is only notified if nothing else,
including the debugger, has handled the exception.
Signed-off-by: Alan Jowett <alanjo@microsoft.com>
Closes#2332Closes#2286Closes#898
This should provide the same overall stdout/err, but the new
output should feel "faster" for test cases that are entered
and exited multiple times (e.g. due to generators).
This requires a bunch of different changes across the reporter
subsystem.
* We need to handle multiple reporters and their differing
preferences in `ListeningReporter`, e.g. what to do when
we mix reporters that capture and don't capture stdout.
* We need to change how the reporter is given output and
how we parse reporter's output destination from CLI.
* Approval tests need to handle multireporter option
By not materializing the lower cased tags ahead of time, we
save allocations at the cost of worsened performance when comparing
two tags.
Since there are rarely many tags, and commonly they are not
compared even if present, this is almost always a win. The new
implementation also improves the robustness of the code
responsible for handling tags in a case-insensitive manner.
A new flag, `--allow-running-no-tests` was added to override this
behaviour if exit code of 0 was desired.
This change also made `-w NoTests` obsolete, and so it has been
removed.
All of its functionality can be moved into the `MatcherBase` class,
simplifying the code a bit and removing a warning about class with
virtual member functions but no virtual destructor.
Closes#2182 as it is no longer relevant.
Previously registration was case preserving, but lookup used
lowercased reporter name, so a reporter whose name contained
upper case character could not be requested by the user.
The problem was that every line would iterate from current line
start position to the end of the string, looking for a newline
to break on, leading to accidentally quadratic runtime. With this
change, the code only ever searches up to the current line's
length and not more.
Credit to @jorgenpt for the fix suggestion.
Closes#2315
This is what should normally happen, even if it does not change
anything given that `Column::const_iterator` is currently a typedef
for `Column::iterator`.
Previously a lambda parser in Clara could only be invoked once,
even if it internally was ok with being invoked multiple times.
With this change, a lambda parser can mark itself as `accept_many`,
in which case it will be invoked multiple times if the appropriate
flag was supplied multiple times by the user.
This greatly simplifies running Catch2 tests in single binary
in parallel from external test runners. Instead of having to
shard the tests by tags/test names, an external test runner
can now just ask for test shard 2 (out of X), and execute that
in single process, without having to know what tests are actually
in the shard.
Note that sharding also applies to test listing, and happens after
tests were ordered according to the `--order` feature.
The problem came from the console reporter trying to provide a
fancy linebreaking (primarily for things like `SCENARIO` or the
BDD macros), so that new lines start with extra indentation if
the text being line broken starts as "{text}: ".
The console reporter did not properly take into account cases
where the ": " part would already be in a later line, in which
case it would ask for non-sensical level of indentation (larger
than single line length).
We fixed this by also enforcing that the special indentation case
only triggers if the ": " is found early enough in the line, so
that we also avoid degenerate cases like this:
```
blablabla: F
a
n
c
y
.
.
.
```
Fixes#2309
Clang-tidy is smart enough to understand that the conditional is never
updated in the loop body. It will let you get away with it if it can
prove that the conditional is always false, but that is not always
possible.
Here is an example where it's not able to prove it, and thus gives a
false positive. This is a minimal reproduction of an actual case I hit
in production, where `function` is picking the function based on some
`constexpr` logic related to which type argument is currently being
tested.
```
int f();
TEMPLATE_TEST_CASE("reproduction", "", int) {
const auto function = []() {
return f;
}();
const int error = function();
REQUIRE(error == 0); // clang-tidy complains: bugprone-infinite-loop
}
```
I did not choose to add this test to the test suite, since we're not
running `clang-tidy` in CI afaik. To run it manually, simply add the
snippet above somewhere and run clang-tidy with
`--checks=bugprone-infinite-loop`. Or see an example at
https://godbolt.org/z/4v8b8WexP.
The reason we get the infinite loop warning in the first place is the
conditional at the end of this `do`-loop. Ideally, this conditional
would just be `while(false)`, but the actual content of the
`REQUIRE`-statement has been included here too in order to not loose
warnings from signed/unsigned comparisons. In short, if you do
`REQUIRE(i < j)`, where `i` is a negative signed integer and `j` is an
unsigned integer, you're supposed to get a warning from
`-Wsign-compare`. Due to the decomposition in Catch2, you lose this
warning, which is why the content of the `REQUIRE` statement has been
added to the conditional to force the compiler to evaluate the actual
comparison as well.
This was discussed on Discord today, and an alternative approach (which
I don't have time to implement) would be to in the decomposition replace
the comparison operators with `cmp_less` and friends. These are C++20
though, and would have to be implemented manually. I am also not sure
it's a good idea to "magically" change the semantics of `<` when it's
used inside a `REQUIRE` macro.
Another alternative approach would be to trigger this warning in a
different way, by including the content of the `REQUIRE` macro in a
different way which doesn't affect the for loop. But I don't have enough
of an overview here to know where would be a good place and how to test
that I didn't break anything.
We used to use whatever precision we ended up having from C++'s
stdlib. However, some relatively popular tools, like Jenkins,
use Maven SureFire XML schema to validate JUnit test reports, and
Maven SureFire schema requires the duration to have at most 3
decimal places.
For compatibility, the JUnit reporter will now respect this
limitation.
Closes#2221
There is no good reason to provide a "add empty line" primitive
for writing XML documents, and the fact that it remains unused
after all the time it was provided only confirms this further.
This decreases code size and improves performance of passing around
`unique_ptr` instances by value somewhat. It virtually guarantees
problems when combining code compiled with Clang and GCC, but that
was never supported anyway.
In b7b346c3e5 this conditional was simplified to just
`while( false)` rather than the current one. Then in 3a33315ff8, that
change was reverted. I found it hard to understand this
complicated conditional, but after some digging in history I found this
comment which used to be here. It was removed in b7b346c3e5, but not
restored together with the revert in 3a33315ff8. Let's revive it.
It used to be part of the experimental benchmarking support, but
since that was replaced with proper benchmarking support with its
own timer facilities, it is now a dead code and useless.
The problem with the old name was that it collided with the
range matcher `Contains`, and it was not really possible to
disambiguate them just with argument types.
Closes#2131
This change also changes it so that test case macros using a
class name can have same name **and** tags as long as the
used class name differs.
Closes#1915Closes#1999
They now take `StringRef` as the argument, and are virtual only
in the basic interface.
Also cleaned out the various reporters and their overrides
of these members which were often empty or delegating up.
This means that e.g. for `TEST_CASE` with two sibling `SECTION`s
the event will fire twice, because the `TEST_CASE` will be entered
twice.
Closes#2107 (the event mentioned there already exists, but this
is its counterpart that we also want to provide to users)
This means that it can no longer be safely made ahead of time,
but nothing in our existing code used it like that. Normally it
is constructed and used in the same expression, which is now
more efficient.
Using the `CATCH_MOVE` and `CATCH_FORWARD` macros instead of the
`std::move` and `std::forward<T>` utility functions can improve
compilation times and debug build's performance, and thus will
be preferred going forward.
With these changes, all these benchmarks
```cpp
BENCHMARK("Empty benchmark") {};
BENCHMARK("Throwing benchmark") {
throw "just a plain literal, bleh";
};
BENCHMARK("Asserting benchmark") {
REQUIRE(1 == 2);
};
BENCHMARK("FAIL'd benchmark") {
FAIL("This benchmark only fails, nothing else");
};
```
report the respective failure and mark the outer `TEST_CASE` as
failed. Previously, the first two would not fail the `TEST_CASE`,
and the latter two would break xml reporter's formatting, because
`benchmarkFailed`, `benchmarkEnded` etc would not be be called
properly in failure cases.
This is a simplification of the fix proposed in #2152, with the
critical function split out so that it can be tested directly,
without having to go through the ULP matcher.
Closes#2152
In v2 it was placed in a very central header due to the way it was
stitched together. Now that we don't do that, we can move it to the
proper place, removing the potential for confusion given that the
original header was split apart and renamed.
- NVHPC's implementation of `__builtin_constant_p` has a bug which
results in calls to the immediately evaluated lambda expressions to be
reported as unevaluated lambdas.
https://developer.nvidia.com/nvidia_bug/3321845.
- Hence, we disable CATCH_INTERNAL_IGNORE_BUT_WARN for NVHPC Compilers
This let's us avoid running `strlen` at runtime to convert the
plain string literals to `StringRef`s, by guaranteeing that we
instead have the size available after compilation.
In optimized builds the performance improvement should be even
greater, as the `StringRef` UDL and the related constructor
are both `constexpr`, and thus can be baked completely during
compilation.
Previously, string literals and `std::string`s would match the
template variant, which would serialize them into a stream and then
call the `StringRef` overload for resulting string. This caused
bunch of codebloat and unnecessary pessimization for common usage.
This introduces a potential lifetime risk when using the API, but
the intended way to use the `XmlEncode` class is to use it directly,
e.g. `out << XmlEncode(some-text-argument)`, not to store it around.
The benefit is that we avoid allocations for strings that do not fit
into SSO for given platform.
In some places the `std::flush` was not added, as it was sufficiently
obvious that the flush semantics are not intended. There are likely
other places where the flush semantics aren't intended, but that
is a cleanup for later.
More specifically, made the actual implementation of string-like
type handling take argument as `Catch::StringRef`, instead of
taking `std::string const&`.
This means that string-like types that are not `std::string` no
longer need to pay for an extra construction of `std::string`
(including the potential allocation), before they can be stringified.
The actual string stringification routine is now also better about
reserving sufficient space.
Apart from being clearer, it also improves the overall codesize
of the implementation library, and should improve the performance
as well, by removing one level of indirection.
Because new glibc has changed `MINSIGSTKSZ` to be a syscall instead
of being constant, the signal posix handling needed changes, as it
used the value in constexpr context, for deciding size of an array.
It would be simple to fix it by having the handler determine the
signal handling stack size and allocate the memory every time the
handler is being installed, but that would add another allocation
and a syscall every time a test case is entered.
Instead, I split apart the idea of preparing fatal error handlers,
and engaging them, so that the memory can be allocated only once
and still be guarded by RAII.
Also turns out that Catch2's use of `MINSIGSTKSZ` was wrong, and
we should've been using `SIGSTKSZ` the whole time, which we use now.
Closes#2178
* [Issue 2154] Correct error when building with IBM's latest XLC compiler with xlclang++ front-end.
On AIX, the XLC 16.1.0.1 compiler considers the call to `std::abs` ambigious, so it needs help with a static_cast to the type of the template argument.
Co-authored-by: Martin Hořeňovský <martin.horenovsky@gmail.com>
Also generalized the implementations to write to the provided
output stream, which will be required for the follow up changes,
where the listings should happen to the location user asked for
by specifying the `-o` flag.
Previously, every base derived from the IStreamingReporter had
its own `IConfig const* m_config` member, so this just centralizes
the handling thereof.
Part of #2061
The old name was a legacy of v2 era, where all headers were stitched
into one. With v3 using separate headers, it is better when they have
proper name.
Previous splitting of catch_common.hpp left it containing only one
actual thing, which is the `SourceLineInfo` type. Given that, there
is no reason to keep the old name.
Also found out that it was included in some places for no reason
(primarily Matchers).
With GCC 10, the `static_cast<bool>` triggers the -Wuseless-cast warning. This commit changes the cast into `static_cast<const bool&>`: it achieves the same thing but doesn't trigger the warning thanks to the "gratuitous" type conversion to `const bool&`. As per references rules, `const bool&` should bind to anything, be it `const` or not, an rvalue or an lvalue, so I doubt that this change is breaking anything.
The problem was that under specific circumstances, namely that none
of their children progressed, `GeneratorTracker` will not progress.
This was changed recently, to allow for code like this, where a
`SECTION` follows a `GENERATE` at the same level:
```cpp
SECTION("A") {}
auto a = GENERATE(1, 2);
SECTION("B") {}
```
However, this interacted badly with `SECTION` filters (`-c foo`),
as they could deactivate all `SECTION`s below a generator, and thus
stop it from progressing forever. This commit makes GeneratorTracker
check whether there are any filters active, and if they are, it checks
whether its section-children can ever run.
Fixes#2025
The new output (mostly) follows the old `--list-test-names-only`
format, with the exception of no longer supporting line output
for `--verbosity high`.
Fixes#2051
The problem was that Catch2 did not reliably include `<exception>`
before it checked for the feature test macro for
`std::uncaught_exceptions`. To avoid overhead of including
`<exception>` everywhere, the configuration check was split out
into a separate header.
Closes#2021
As far as I understand the standard, if there is a function called
`rng` in the global namespace, and a function argument called `rng`,
then the argument should shadow the function. This then means that
uses of `rng` inside the function should refer to the argument.
This is not the case for AppleClang 12.0.0. Luckily the workaround
is simple enough; just rename the argument. Given that the function
is 3 lines and uncomplicated, the change of the name doesn't really
affect readability.
Still, WTF AppleClang?
Closes#2030
At some places, the colour reset code is printed after a newline.
Since the default output buffering to console is line-based, the reset
code is not actually written out. If messages from user code are printed
to stderr (different stream, same console), they are printed before
the colour reset code, and thus they are coloured.
Explicitly flushing the stream after writing the colour escape code solves
this.
Most of the changes are completely pointless renaming of constructor
arguments so that they do not use the same name as the type members,
but 🤷Closes#2015
This commit also strips the old copyright comment header in touched
files, as those will also be replaced with a more standardized and
machine-friendly version.
The base was also renamed from `TestEventListenerBase` to
`EventListenerBase`, and modified to derive directly from the
reporter interface, rather than deriving from `StreamingReporterBase`.
Due to also adding a new TU, there is no improvement to the
compilation times of the static library, but it improves the
compilation times of consumer's reporter TUs.
Doing this removes `<map>` from the include set of the base reporter
interface, and thus from bunch more TUs. This provides about 1.5%
improvements in the debug build of the static library, and 1% in
release build.
Each of the two reporter bases now has its own header file, and
cpp file. Even though this adds another TU to the compilation,
the total CPU time taken by compilation is reduced by about 1%
for debug build and ~0.5% for optimized build of the main library.
(The improvement would be roughly doubles without splitting the TUs,
but the maintainability hit is not worth it.)
The code size of the static library build has also somewhat decreased.
Follow up: Introduce combined TU for reporters, and further split
apart the catch_reporter_streaming_base.hpp header into its
constituent parts, as it still contains a whole bunch of other stuff.
Anchoring the vtables does 2 things
1) Fixes some instances of `-Wweak-vtables`
2) Decreases code size and linker pressure
However, there are still some unanchored ones, and thus we have
to keep suppressing `-Wweak-vtables` warning for Clang.
* Added some missing `noexcept`s on custom destructors.
* Fixed `std::move` being called on a const-reference.
* Initialized `ScopedMessage::m_moved` in class definition, instead
of doing so in constructors explicitly.
* Turned some `enum`s into `enum class`es.
* Initialized `StreamingReporterBase::currentTestCaseInfo` in class
definition.
* Some cleanups in SelfTest code.
The includes were an artifact of the old design where the built-in
reporter TUs used autoregistration to add the reporters to the
factory. Because the current design is to add them explicitly in
the central reporter factory TU, the includes are useless.
This saves a tiny little bit of compilation times when the
`ApproxMatcher` is used and `epsilon`, `margin`, or `scale` are
used to customize its behaviour.
As the full `Config` is not needed, the TUs implementing the `list*`
functions can require the less heavy header `catch_interfaces_config.hpp`
instead of the much heavier `catch_config.hpp`.
This commit also fixes up some other TUs that include `Config`,
while using just `IConfig`, to cleanup the includes further.
* Clara is now split between a header and a cpp file.
* Removed the deprecated `+` and `+=` operators for composing
a parser.
* Renamed `clara` and `detail` namespaces to be inline with the
rest of Catch2 (they are now `Clara` and `Detail` respectively).
* Taken most of user-exposed types out of the `Detail` namespace
completely (instead of using `using` directives to bring them into
the outer namespace).
Now that it has its own header, various reporter TUs that want to
format text do not have to also include Clara. Together with
outlining implementations from a header into a separate TU, this
has noticeably improved the compilation times of the testing impl.
As part of this split, I also implemented some improvements to the
TextFlow code in comparison to the upstream code. These are:
* Replaced the `Spacer` type with a free function that constructs
special `Column` that does the same thing.
* Generic performance improvements, such as eliminating needless
allocations, reserving space in needed allocations, and using smarter
algorithms in some places.
* Because `Column` only ever stored 1 string in its vector, it now
holds the string directly instead.
This means that code that uses it no longer has to include all of
catch_config.hpp, which seems to provide significant reduction in
size of unoptimized compilation of the final static library.
This is not nearly all of them, because IWYU does not support the
way Catch2 manages includes -- it expects that non-system includes
are done using `#include "foo/bar/baz.hpp"`, while Catch2 uses
`<foo/bar/baz.hpp>`. This causes trouble, because IWYU suggests
removing every single internal header, and then adding them again,
but using `""` in the include directive... the resulting suggestions
cannot be used without a lot of manual work, as they are largely
bogus.
For bonus points, IWYU also _loves_ to suggest kinda-random stdlib
headers for `size_t` and similar. Still, the resulting inclusion
graph is somewhat better than it was before.
The new scheme is that there is one protected member instance of
`ReporterPreferences` in the `IStreamingReporter` base class,
and derived classes can modify it to express their own preferences.
Retrieving the preferences is now a non-virtual operation, which
makes it much cheaper to read them frequently. Previously, we
avoided doing so by caching the preferences in another variable,
but we still read them at least once per test case run.
