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

[Frediano Ziglio]

>
> 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/Makefile.am             | 12 +++++++++
>  src/unittests/test-mjpeg-fallback.cpp | 50
>  +++++++++++++++++++++++++++++++++++
>  5 files changed, 71 insertions(+)
>  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)])])
> +
>  dnl
>  ===========================================================================
>  dnl check compiler flags
>  

I like the idea that tests have to succeed but I'm thinking about RHEL7
(which we need to support) and how to manage it taking into account that
catch is currently not packaged there.

Is it currently in epel 7 packages but I don't know if is possible to
use epel 7 in our case.

Maybe instead of the error we could have it disabled?

> diff --git a/src/mjpeg-fallback.cpp b/src/mjpeg-fallback.cpp
> index 7aaa355..d6dcf74 100644
> --- a/src/mjpeg-fallback.cpp
> +++ b/src/mjpeg-fallback.cpp
> @@ -186,6 +186,11 @@ void MjpegPlugin::ParseOptions(const ConfigureOption
> *options)
>      }
>  }
>  
> +MjpegSettings MjpegPlugin::Options() const
> +{
> +    return settings;
> +}
> +
>  SpiceVideoCodecType MjpegPlugin::VideoCodecType() const {
>      return SPICE_VIDEO_CODEC_TYPE_MJPEG;
>  }
> diff --git a/src/mjpeg-fallback.hpp b/src/mjpeg-fallback.hpp
> index 0e9ed6a..65c5fd3 100644
> --- a/src/mjpeg-fallback.hpp
> +++ b/src/mjpeg-fallback.hpp
> @@ -24,6 +24,7 @@ public:
>      FrameCapture *CreateCapture() override;
>      unsigned Rank() override;
>      void ParseOptions(const ConfigureOption *options);
> +    MjpegSettings Options() const;  // TODO unify on Settings vs Options
>      SpiceVideoCodecType VideoCodecType() const;
>      static bool Register(Agent* agent);
>  private:

What do you mean with the TODO comment?

> diff --git a/src/unittests/Makefile.am b/src/unittests/Makefile.am
> index ef6c253..be25a0e 100644
> --- a/src/unittests/Makefile.am
> +++ b/src/unittests/Makefile.am
> @@ -2,6 +2,7 @@ NULL =
>  
>  AM_CPPFLAGS = \
>  	-DRH_TOP_SRCDIR=\"$(abs_top_srcdir)\" \
> +	-I$(top_srcdir)/include \
>  	-I$(top_srcdir)/src \
>  	-I$(top_srcdir)/src/unittests \
>  	$(SPICE_PROTOCOL_CFLAGS) \
> @@ -14,10 +15,12 @@ AM_CFLAGS = \
>  
>  check_PROGRAMS = \
>  	test-hexdump \
> +	test-mjpeg-fallback \
>  	$(NULL)
>  
>  TESTS = \
>  	hexdump.sh \
> +	test-mjpeg-fallback \
>  	$(NULL)
>  
>  noinst_PROGRAMS = \
> @@ -32,6 +35,15 @@ test_hexdump_LDADD = \
>  	../libstreaming-utils.a \
>  	$(NULL)
>  
> +test_mjpeg_fallback_SOURCES = \
> +	test-mjpeg-fallback.cpp \
> +	../mjpeg-fallback.cpp \
> +	../jpeg.cpp
> +
> +test_mjpeg_fallback_LDADD = \
> +	$(X11_LIBS) \
> +	$(JPEG_LIBS)
> +
>  EXTRA_DIST = \
>  	hexdump.sh \
>  	hexdump1.in \
> diff --git a/src/unittests/test-mjpeg-fallback.cpp
> b/src/unittests/test-mjpeg-fallback.cpp
> new file mode 100644
> index 0000000..08ed406
> --- /dev/null
> +++ b/src/unittests/test-mjpeg-fallback.cpp
> @@ -0,0 +1,50 @@
> +#define CATCH_CONFIG_MAIN
> +#include "catch/catch.hpp"
> +
> +#include "mjpeg-fallback.hpp"
> +
> +namespace ssa = SpiceStreamingAgent;
> +
> +
> +TEST_CASE("test parse options", "[mjpeg][options]")
> +{
> +    ssa::MjpegPlugin plugin;
> +
> +    SECTION("correct setup") {
> +        std::vector<ssa::ConfigureOption> options = {
> +            {"framerate", "20"},
> +            {"mjpeg.quality", "90"},
> +            {NULL, NULL}
> +        };
> +
> +        plugin.ParseOptions(options.data());
> +        ssa::MjpegSettings new_options = plugin.Options();
> +        CHECK(new_options.fps == 20);
> +        CHECK(new_options.quality == 90);
> +    }
> +
> +    SECTION("unknown option") {
> +        std::vector<ssa::ConfigureOption> options = {
> +            {"wakaka", "10"},
> +            {NULL, NULL}
> +        };
> +
> +        REQUIRE_THROWS_WITH(
> +            plugin.ParseOptions(options.data()),
> +            "Invalid option 'wakaka'."
> +        );
> +    }
> +
> +    SECTION("invalid option value") {
> +        std::vector<ssa::ConfigureOption> options = {
> +            {"framerate", "10"},
> +            {"mjpeg.quality", "toot"},
> +            {NULL, NULL}
> +        };
> +
> +        REQUIRE_THROWS_WITH(
> +            plugin.ParseOptions(options.data()),
> +            "Invalid value 'toot' for option 'mjpeg.quality'."
> +        );
> +    }
> +}

Frediano