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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 2018-02-14 at 16:08 +0100, Christophe de Dinechin wrote:
> > On 14 Feb 2018, at 13:34, Lukáš Hrázký <lhrazky@xxxxxxxxxx> 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@xxxxxxxxxx>
> > ---
> > 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?

Unless this is a real obstacle for someone, I'd rather just force the
unittests on people, otherwise, nobody (except for us I suppose) will
care for the warning if everything still works...

> > +
> > dnl ===========================================================================
> > dnl check compiler flags
> > 
> > diff --git a/src/mjpeg-fallback.cpp b/src/mjpeg-fallback.cpp
> > index 1b51ee0..ae1777d 100644
> > --- a/src/mjpeg-fallback.cpp
> > +++ b/src/mjpeg-fallback.cpp
> > @@ -181,6 +181,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 04fa2eb..8496763 100644
> > --- a/src/mjpeg-fallback.hpp
> > +++ b/src/mjpeg-fallback.hpp
> > @@ -25,6 +25,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;
> > private:
> >     MjpegSettings settings = { 10, 80 };
> > diff --git a/src/unittests/.gitignore b/src/unittests/.gitignore
> > index af41c48..22f1335 100644
> > --- a/src/unittests/.gitignore
> > +++ b/src/unittests/.gitignore
> > @@ -1,4 +1,5 @@
> > /hexdump
> > -/test-hexdump.sh.log
> > -/test-hexdump.sh.trs
> > +/test-*.log
> > +/test-*.trs
> > +/test-mjpeg-fallback
> > /test-suite.log
> > diff --git a/src/unittests/Makefile.am b/src/unittests/Makefile.am
> > index a70a4b4..bd079c3 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 = \
> > 	hexdump \
> > +	test-mjpeg-fallback \
> > 	$(NULL)
> > 
> > TESTS = \
> > 	test-hexdump.sh \
> > +	test-mjpeg-fallback \
> > 	$(NULL)
> > 
> > noinst_PROGRAMS = \
> > @@ -32,6 +35,18 @@ hexdump_LDADD = \
> > 	../libstreaming-utils.a \
> > 	$(NULL)
> > 
> > +test_mjpeg_fallback_SOURCES = \
> > +	test-mjpeg-fallback.cpp \
> > +	../jpeg.cpp \
> > +	../mjpeg-fallback.cpp \
> > +	../static-plugin.cpp \
> > +	$(NULL)
> > +
> > +test_mjpeg_fallback_LDADD = \
> > +	$(X11_LIBS) \
> > +	$(JPEG_LIBS) \
> > +	$(NULL)
> > +
> > EXTRA_DIST = \
> > 	test-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..4a152fe
> > --- /dev/null
> > +++ b/src/unittests/test-mjpeg-fallback.cpp
> > @@ -0,0 +1,58 @@
> > +#define CATCH_CONFIG_MAIN
> > +#include "catch/catch.hpp"
> > +
> > +#include "mjpeg-fallback.hpp"
> > +
> > +namespace ssa = spice::streaming_agent;
> > +
> > +
> > +SCENARIO("test parsing mjpeg plugin options", "[mjpeg][options]") {
> > +    GIVEN("A new mjpeg plugin") {
> > +        ssa::MjpegPlugin plugin;
> > +
> > +        WHEN("passing correct options") {
> > +            std::vector<ssa::ConfigureOption> options = {
> > +                {"framerate", "20"},
> > +                {"mjpeg.quality", "90"},
> > +                {NULL, NULL}
> > +            };
> > +
> > +            plugin.ParseOptions(options.data());
> > +            ssa::MjpegSettings new_options = plugin.Options();
> > +
> > +            THEN("the options are set in the plugin") {
> > +                CHECK(new_options.fps == 20);
> > +                CHECK(new_options.quality == 90);
> > +            }
> > +        }
> > +
> > +        WHEN("passing an unknown option") {
> > +            std::vector<ssa::ConfigureOption> options = {
> > +                {"wakaka", "10"},
> > +                {NULL, NULL}
> > +            };
> > +
> > +            THEN("ParseOptions throws an exception") {
> > +                REQUIRE_THROWS_WITH(
> > +                    plugin.ParseOptions(options.data()),
> > +                    "Invalid option 'wakaka'."
> > +                );
> > +            }
> > +        }
> > +
> > +        WHEN("passing an invalid option value") {
> > +            std::vector<ssa::ConfigureOption> options = {
> > +                {"framerate", "40"},
> > +                {"mjpeg.quality", "toot"},
> > +                {NULL, NULL}
> > +            };
> > +
> > +            THEN("ParseOptions throws an exception") {
> > +                REQUIRE_THROWS_WITH(
> > +                    plugin.ParseOptions(options.data()),
> > +                    "Invalid value 'toot' for option 'mjpeg.quality'."
> > +                );
> > +            }
> > +        }
> > +    }
> > +}
> > -- 
> > 2.16.1
> > 
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> 
> 
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]