Skip to content
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 a possibility to return a default value when there are no matches #257

Open
u3shit opened this issue Oct 2, 2021 · 7 comments
Open

Comments

@u3shit
Copy link

u3shit commented Oct 2, 2021

Basically I have a bunch of noexcept functions, and I've been looking for something to at least not crash the whole testing process because of throwing in a noexcept function. Would it be possible to somehow specify a default return value for a mocked function? So basically when reporting a mismatch, it could be done with severity::nonfatal and then returning the dummy value, while still marking the test case as failed. I can't simply ALLOW_CALL with a dummy return value either, because in that case it will count as a success.
As a workaround right now I'm using something like this

ALLOW_CALL(obj, foo(_)).
        SIDE_EFFECT(DOCTEST_ADD_FAIL_CHECK_AT("??", 0, "Called foo")).
        RETURN(0);

which at least allows the test case to continue and not crash. However, other than being doctest specific (that could be solved, it would just have to call trompleloeil's reporter directly somehow), this currently doesn't report parameters or print other matchers the function might have. (And it's a bit long, but I could turn it into a macro to somewhat shortens it).

@rollbear
Copy link
Owner

rollbear commented Oct 2, 2021

It would be a convenience, but a very dangerous one, because you so easily lose track of what's allowed and what's not. I've spent uncountable hours fixing tests that use this feature in other frameworks, passing tests for code that's trivially wrong. In fact, I consider the lack of a forgiving default to be a feature, one of the fundaments on which Tromploeil stands, because it forces you to be explicit about what you mean.

@u3shit
Copy link
Author

u3shit commented Oct 2, 2021

Maybe I wasn't clear enough, it would still generate a failure and fail the test, it would just return some value so your test framework could handle the rest instead of crashing with an abort() because of uncaught exceptions.

@rollbear
Copy link
Owner

rollbear commented Oct 3, 2021

Ah, I see, I misunderstood. Sorry. Are you using the provided adapter for doctest, by using #include <doctest/trompeloeil.hpp>?

It will report all violations using doctest's reporting mechanisms, and does not abort due to uncaught exceptions.

Example:

https://godbolt.org/z/d8o7YqbWY

@u3shit
Copy link
Author

u3shit commented Oct 3, 2021

Yes, I'm using the provided test adapter. The problem is that with a fatal failure, the provided reporter will call DOCTEST_ADD_FAIL_AT, which still throws at the end (it creates an is_require assert here:
https://github.com/onqtam/doctest/blob/4d8716f1efc1d14aa736ef52ee727bd4204f4c40/doctest/doctest.h#L2053-L2063
which then throws an exception later:
https://github.com/onqtam/doctest/blob/4d8716f1efc1d14aa736ef52ee727bd4204f4c40/doctest/doctest.h#L4569-L4570
unless doctest is built with DOCTEST_CONFIG_NO_EXCEPTIONS, but it has their own drawbacks: https://github.com/onqtam/doctest/blob/4d8716f1efc1d14aa736ef52ee727bd4204f4c40/doc/markdown/configuration.md#doctest_config_no_exceptions). And since the reporter shouldn't return when called with a fatal severity, it can't really do anything other than throwing an exception or aborting/exiting.

In your example, if I replace

MAKE_MOCK1(foo, void(int));

with

MAKE_MOCK1(foo, void(int), noexcept);

it will crash with a SIGABRT.

@rollbear
Copy link
Owner

rollbear commented Oct 4, 2021

This is a bad situation, and I don't know how to handle it. You really don't want to return here. It would solve the immediate problem of crashing the test runner, but it would also mean that the test would continue doing bad things after a known violation. Maybe a total rewrite of the reporting logic to use longjmp() shenanigans could be made to work, but invoking undefined behaviour in those situations is far easier than not to. I will need to think about this. There is no easy solution that I can think of right now.

@u3shit
Copy link
Author

u3shit commented Oct 4, 2021

Yes, I agree that longjmp is bad with C++, as soon as you cross a stackframe with an object with a destructor, you're already invoking undefined behavior. And if nothing else, ALLOW_CALL and friends already place objects with destructor on the stack...

Originally I wanted to define a default value per mocked function, so a developer could decide which is worse, crashing the test runner, or allowing the code to continue with some dummy returned value on a case-by-case basis. Maybe it's a bit overkill, but I don't have a generic solution to this problem either.

@rollbear
Copy link
Owner

Marked this as "help wanted". To be honest, I don't think this is currently possible to address without causing even worse problems, but I'm always open to the possibility that there's a way I haven't thought of.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants