Code review comment for lp:~andreas-pokorny/mir/load-all-supported-input-platforms

Revision history for this message
Chris Halse Rogers (raof) wrote :

+ mir::log_info("Found input driver: %s", desc->name);

Could you also log driver version please?

+ return std::move(platforms);

Pessimisation: this breaks return value optimisation. When the Wily clang builds work again this will be an error.

+ EXPECT_THAT(platforms.size(), Eq(2)); // we cannot disable the default evdev platform with only umock dev is in place
+ EXPECT_THAT(platforms[0].get(), AnyOf(OfType<mi::evdev::Platform>(), OfType<mi::X::XInputPlatform>()));
+ EXPECT_THAT(platforms[1].get(), AnyOf(OfType<mi::evdev::Platform>(), OfType<mi::X::XInputPlatform>()));

Seems like you could replace OfType with OfPointerType and then use

EXPECT_THAT(platforms, UnorderedElementsAre(OfPointerType<mi::evdev::Platform>(), OfPointerType<mi::X::XInputPlatform>());

Which I think is both more natural and also is a slightly stronger test.

Non-review comment:
+ mtf::UdevEnvironment env; // TODO replace with mock_libevdev und mock_libudev when the new input reading stack is the only in use.

Heads up: I think that doing this will provide negative value. Also, und→and :)

review: Needs Fixing

« Back to merge proposal