Mir

Code review comment for lp:~vanvugt/mir/resize-events

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

> Alan,
>
> I don't think that's a valid argument. You can comment out
> asserts/expectations from any tests and they will of course pass.
>
> The point is to verify send is never called at all for those event types. And
> that it is called for other event types. I just re-tested changing "Times(0)"
> to "Times(1)" and the test then fails. So the test is correct.

Sorry, I wasn't clear.

The canonical way to write a test is /1/ setup, /2/ expectations, /3/ execution and /4/ validation.

You've mixed expectations and execution - but that doesn't change the way the fact that validation only occurs at the end. Essentially you've written:

TEST(TestEventSender, sends_all_but_input_events)
{
    using namespace testing;

    // Setup
    auto msg_sender = std::make_shared<MockMsgSender>();
    mfd::EventSender event_sender(msg_sender);
    MirEvent event;
    memset(&event, 0, sizeof event);

    // Expectations
    InSequence seq;
    EXPECT_CALL(*msg_sender, send(_))
        .Times(2);

    // Execution
    event.type = mir_event_type_surface;
    event_sender.handle_event(event);
    event.type = mir_event_type_resize;
    event_sender.handle_event(event);
    event.type = mir_event_type_key;
    event_sender.handle_event(event);
    event.type = mir_event_type_motion;
    event_sender.handle_event(event);
}

Which doesn't give any information on which events generate which calls - just that there are two of them.

« Back to merge proposal