[Spice-devel] [PATCH spice-streaming-agent 2/4] mjpeg-fallback: unittest for the options parsing
Lukáš Hrázký
lhrazky at redhat.com
Thu Feb 1 15:28:39 UTC 2018
On Thu, 2018-02-01 at 10:13 -0500, Frediano Ziglio wrote:
> >
> > On Thu, 2018-02-01 at 06:24 -0500, Frediano Ziglio 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/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.
> >
> > That's what I was afraid of.
> >
> > > Maybe instead of the error we could have it disabled?
> >
> > I don't think it's a good idea to disable unit tests on a platform
> > we
> > care about. Another option is to use gtest (which I suppose is in
> > RHEL?
> > How to find out?) though I was liking Catch so far.
> >
> > Lukas
> >
>
> Yes, don't like to disable the tests too.
>
> There could be different options:
> - we push to get the package (catch-devel) into RHEL. Not sure how
> much this could take;
> - if not too big we include catch library into a patch to include
> inside the RHEL package. Not sure how this is easy to do.
I don't think it would be hard at all to package it for RHEL. It's just
a header-only library and actually has no dependencies whatsoever.
So, how do we go about getting it in? :)
Lukas
> > > > 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'."
> > > > + );
> > > > + }
> > > > +}
More information about the Spice-devel
mailing list