Code review comment for lp:~mir-team/mir/attestable-timestamps-server

>
> +std::string mir_test_framework::udev_recordings_path()
> +{
> + std::string bin_path = MIR_BUILD_PREFIX"/bin/udev_recordings";
> + std::string share_path = MIR_INSTALL_PREFIX"/share/udev_recordings";
> +
> + if (boost::filesystem::exists(bin_path))
> + return bin_path;
> + else if (boost::filesystem::exists(share_path))
> + return share_path;
> +
> + BOOST_THROW_EXCEPTION(std::runtime_error("Failed to find udev_recordings
> in standard search locations"));
> +}
>
> Can this really be the right behaviour? An installed test binary looks in the
> location where it was built? Even though a developer may be making changes
> there to compare results with.

I should check the share folder first. Other then that... I think its correct behaviour. We still want to be able to test it in a build directory. Unless I add an env variable that we set for the test it self. Let me know which one you would like!

>
> ~~~~
>
> + char const* RANDOM_DEVICE_PATH = "/dev/random";
> + int const WAIT_SECONDS = 30;
> ...
> + int const MAX_WAIT = 4;
>
> http://unity.ubuntu.com/mir/cppguide/index.html?showone=Constant_Names#Constan
> t_Names
>

I need to read through the coding standard for mir :). Use to compiz/unity7/nux.

> ~~~~
>
> AFAICS libmircookie is only used by libmirserver.
>
> What is the advantage to having a shared library over linking the code
> directly into libmirserver?
>

This is how it currently stands but we are planning on using this for the content hub as well. So best to keep it shared now, as it will be used by something else.

> ~~~~
>
> /mir/tests/acceptance-tests/test_mir_cookie.cpp:39:13: error: unused variable
> 'MAX_WAIT' [-Werror,-Wunused-const-variable]
> int const MAX_WAIT = 4;
> ^
> 1 error generated.

Thanks! Missed removing that.