LR_SIDE_EFFECT(REQUIRE(...)) doesn't work with Catch and GCC about trompeloeil HOT 12 CLOSED

I knew you would like it 😉
Created a separate issue to track.

from trompeloeil.

That is true. I don't know how important it is to fix, however. You can rephrase your test and instead do:

#define FAKE_READING_BYTES(NUM, DATA) \
    REQUIRE_CALL(mock, readWithTimeout(NUM)) \
    .LR_WITH(NUM >= DATA.size()) \
    .RETURN(DATA)

This should have the same effect. If the constraint NUM >= DATA.size()) is not true, the call will not match, and you'll get an error.

from trompeloeil.

BTW, I'm also considering supporting logical operations on matchers. With this, you could rewrite your code as:

#define FAKE_READING_BYTES(NUM, DATA) \
    REQUIRE_CALL(mock, readWithTimeout(trompeloeil::eq(NUM) && trompeloeil::gt(DATA.size())) \
    .RETURN(DATA)

which is kind of neat (or maybe confusingly cute?)

What's your opinion?

from trompeloeil.

Yet an alternative is to venture in to writing a matcher:

auto message(ssize_t size, std::string data)
{
  using trompeloeil::make_matcher;
  return make_matcher<ssize_t>([](ssize_t num, auto& s, auto& str)
			      { return num == s && num >= str.size(); },
			      [](std::ostream& os, auto& s, auto& str)
			      {	os << " == " << s << " for \"" << str << '"'; },
			      size,
			      std::move(data));
}

With this you can change the macro to:

#define FAKE_READING_BYTES(NUM, DATA) \
  REQUIRE_CALL(mock, readWithTimeout(message(NUM, DATA)))	\
    .RETURN(DATA)

and use as before.

A failing case:

TEST_CASE("compiles on g++-6 and clang-3.9") {
    MyMock mock;
    FAKE_READING_BYTES(3, "abc"s);
    REQUIRE(mock.readWithTimeout(2) == "abc");
}

Which gives an error message

t.cpp:32: FAILED:
  REQUIRE( mock.readWithTimeout(2) == "abc" )
due to unexpected exception with message:
  
  No match for call of readWithTimeout with signature std::string(ssize_t)
  with.
    param  _1 == 2
  
  Tried mock.readWithTimeout(message(3, "abc"s)) at t.cpp:31
    Expected  _1 == 3 for "abc"

from trompeloeil.

Thanks for these ideas. What I'm trying to do here is to ensure that the test fixtures won't provide bogus data to the code being tested. That's why I prefer an early fail even before the expectation is checked against a match. I would also like to catch a scenario where the mocked call might not necessarily get called (.TIMES(AT_MOST(1))), and still get a failure.

This is what I currently like the most, despite its macro-induced shortcomings:

#define FAKE_READING_BYTES(NUM, DATA) \
    REQUIRE(NUM >= DATA.size()); \
    REQUIRE_CALL(mock, readWithTimeout(NUM)) \
    .RETURN(DATA)

I understand that it's unsafe/dangerous because it's abusing macros, so concepts like the following one won't work for obvious reasons, but well, there are trade-offs here. I just cannot wrap it in a do { ... } while (false) if I want to be able to use the .SOMETHING(...) syntax.

for (...)
    FAKE_READING_BYTES(3, "abc"s);

I wonder if that's sane from your point of view.

from trompeloeil.

To get rid of the macro, you can write a function that returns a named expectation:

auto fake_reading_bytes(MyMock& mock, ssize_t num, const std::string& data)
{
  REQUIRE(num >= data.size());
  return NAMED_REQUIRE_CALL(mock, readWithTimeout(num))
           .RETURN(data);
}

Which you then bind to a local variable in a test:

  {
    auto read_expectation = fake_reading_bytes(myMock, 3, "abc");
    REQUIRE(myMock, readWithTimeout(3) == "abc");
  }

The disadvantage is that if the test REQUIRE(num >= data.size()) fails, you do not get the best error pinpointing, since __FILE__ and __LINE__ refer into the function and not into the test.

from trompeloeil.

I tried similar thing with boost.test to have more expressive fail messages and it looks awesome:

    REQUIRE_CALL(f.stream, Write(_, _))
        .WITH(_1.has_create_request())
        .SIDE_EFFECT(BOOST_TEST(_1.create_request().key() == "full/key/name1"))
        .SIDE_EFFECT(BOOST_TEST(_1.create_request().start_revision() == 12356))

So when I run I have all of them at once:

test_watch.cpp(163): error: in "etcd_watch/subscribe_full_key": check _1.create_request().key() == "full/key/name1" has failed [full/key/name != full/key/name1]
test_watch.cpp(165): error: in "etcd_watch/subscribe_full_key": check _1.create_request().start_revision() == 12356 has failed [12354 != 12356]

Compared to

REQUIRE_CALL(f.stream, Write(_, _))
        .WITH(_1.has_create_request() && _1.create_request().key() == "full/key/name1")

giving:

error: in "etcd_watch/subscribe_full_key": No match for call of Write with signature void(W const & msg, void * tag) with.
  param  _1 == 32-byte object={
 0x80 0x18 0xb7 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
 0x00 0x76 0xf7 0x10 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x10 0x00 0x00 0x00
 }
  param  _2 == 0x1f526b0

Tried f.stream.Write(_, _) at test_watch.cpp:168
  Failed WITH(_1.has_create_request() && _1.create_request().key() == "full/key/name1")

Probably could be implemented in the mock framework as well (with some SMART_WITH), but use-case works even now.

from trompeloeil.

Hmm. Since the argument to .WITH really needs to be an expression, this can probably be done with expression templates. I'll look into it.

from trompeloeil.

The original issue here works with newer gcc and clang compilers, but only with catch2 2.10.2 and older. catch2 2.11.0 does not work. I suggest writing an issue on catch2 for this.

Closing now because the discussion diverged and the original problem is not a trompeloeil problem.

from trompeloeil.

What if I want to do complex check against MOCK parameter, for examples, casting it to some type, check several fields of that type? How do I achieve this?
I came up with the following:

    REQUIRE_CALL(*mock, send_json(_))
        .IN_SEQUENCE(seq)
        .TIMES(1)
        .SIDE_EFFECT([this, msg = _1] {
            status_notification notif;
            REQUIRE_NOTHROW(notif = boost::any_cast<status_notification>(msg));
            CHECK(notif.car_update_id == 123);
            CHECK(notif.msg == "msg");
        }());

But this does not compile under GCC with #pragma’ is not allowed here, but works just fine with clang.

from trompeloeil.

Please, open a new issue. Also, please, provide a freestanding example that I can work with.

As an aside .TIMES(1), is redundant. REQUIRE_CALL(...) is exactly once unless you want to change it with .TIMES(...).

from trompeloeil.

@rollbear Please, have a look on this #294

from trompeloeil.