-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add exception handling to with test macro #5
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @mattbaumann for the PR, defo worth putting it, ideally though we would remove the header requirements and just catch generic exceptions; we also need some tests but since exceptiosn can't be throw in constexpr we can add it there to show it works as excpected - https://godbolt.org/z/MMPPz1e4a, thanks!
#if __cpp_exceptions < 199711L | ||
#define UT_NO_EXCEPTIONS | ||
#else | ||
#include <exception> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would it be possible not to rely on stl, can we just do throw and catch (const auto& ex) requires requires { ex.waht(); }
?
#endif | ||
test(); | ||
#ifndef UT_NO_EXCEPTIONS | ||
} catch (const char* string) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't we just catch anything which either has what or is printable?
reporter.on(events::exception_fail{ file_name, line, name, string }); | ||
} catch(...) { | ||
try { | ||
std::rethrow_exception(std::current_exception()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think I follow why rethrow is required here, could you elaborate? my initial thought is that catching exception would be the same but I guess I'm missing something?
I propose to add exception handling to ut.
In modern C++ development exceptions are/should be common practice. If a test throws an exception I don't want to abort the whole testing application (all suites that will follow), but to fail that one test only. In the best case, I'd like to see which exception has been thrown to fix things.
I designed it to be fully compliant with -fno-exceptions. The test macro will disable exception handling if not supported by the compiler. Furthermore, exceptions are a runtime-only feature and therefore only implemented for tests running on the silicon.
I know this is a extremely rudimentary implementation, but think the next step should happen in a later pull-request.