[Spice-devel] [PATCH spice-streaming-agent v3 3/3] mjpeg-fallback: unittest for the options parsing

[Uri Lublin]

On 02/14/2018 06:37 PM, Christophe Fergeau wrote:
> On Wed, Feb 14, 2018 at 10:40:58AM -0500, Frediano Ziglio wrote:
>>>
>>>> On 14 Feb 2018, at 13:34, Lukáš Hrázký <lhrazky at redhat.com> wrote:
>>>>
>>>> Introduce a unit test framework (Catch) to the codebase and a simple
>>>> unit test for parsing the options of the mjpeg plugin.
>>>>
>>>> Signed-off-by: Lukáš Hrázký <lhrazky at redhat.com>
>>>> ---
>>>> configure.ac                          |  3 ++
>>>> src/mjpeg-fallback.cpp                |  5 +++
>>>> src/mjpeg-fallback.hpp                |  1 +
>>>> src/unittests/.gitignore              |  5 +--
>>>> src/unittests/Makefile.am             | 15 +++++++++
>>>> src/unittests/test-mjpeg-fallback.cpp | 58
>>>> +++++++++++++++++++++++++++++++++++
>>>> 6 files changed, 85 insertions(+), 2 deletions(-)
>>>> create mode 100644 src/unittests/test-mjpeg-fallback.cpp
>>>>
>>>> diff --git a/configure.ac b/configure.ac
>>>> index 8795dae..5aab662 100644
>>>> --- a/configure.ac
>>>> +++ b/configure.ac
>>>> @@ -17,6 +17,7 @@ if test x"$ac_cv_prog_cc_c99" = xno; then
>>>>      AC_MSG_ERROR([C99 compiler is required.])
>>>> fi
>>>> AC_PROG_CXX
>>>> +AC_LANG(C++)
>>>> AX_CXX_COMPILE_STDCXX_11
>>>> AC_PROG_INSTALL
>>>> AC_CANONICAL_HOST
>>>> @@ -49,6 +50,8 @@ AC_CHECK_LIB(jpeg, jpeg_destroy_decompress,
>>>>      AC_MSG_ERROR([libjpeg not found]))
>>>> AC_SUBST(JPEG_LIBS)
>>>>
>>>> +AC_CHECK_HEADER([catch/catch.hpp],,[AC_MSG_ERROR([Could not find Catch
>>>> dependency header (catch/catch.hpp)])])
>>>
>>> Instead of an error, shouldn’t we instead fallback to not compiling the unit
>>> tests? Possibly a warning?
>>>
>>
>> Good point but I would suggest a follow up and an explicit --I-dont-really-want-unittests
>> option, I agree people should run tests.
>> Another follow up is a patch for the spec file.
> 
> Alternatively, this could go with a --with-catch flag (or
> --enable-unittest or any names which fits you), and
> 1) if there is nothing specified, then we silently enable/disable tests
> depending on the availability of the catch
> 2) if --with-catch is specified, then we error out if it's not found
> 3) if --without-catch is used, then we never use it.
> 
> Then we add --with-catch to autogen.sh, and all developers will have to
> go through extra hoops not to use it.
> 
> Christophe
> 

I agree that's a good alternative.

Uri.