[Spice-devel] [PATCH spice-streaming-agent 3/4] mjpeg-fallback-test: use BDD structure for test cases

[Lukáš Hrázký]

On Thu, 2018-02-01 at 04:17 -0500, Frediano Ziglio wrote:
> > 
> > Behaviour Driven Development defines self-describing, story-like
> > structures for testing that at the same time document them and
> > descriptive messages when a test fails.
> > 
> > Signed-off-by: Lukáš Hrázký <lhrazky at redhat.com>
> 
> More or less the same, once you learn the different macros I
> don't think there's much difference.
> The only big difference I can see is that this syntax requires
> more indentation.
> 
> What do you prefer and why?

I think I prefer the BDD version in this patch. While a bit more
verbose and adding quite a bit of indentation, the benefit is the more
detailed description of the test scenario built into the macros
themselves. So it:

1) Forces the developer to write it and also forces the format, as
opposed to random comments that some people would write, but in
different ways.

2) When the test fails (you can try it if you run `make check` without
patch 4/4), it prints out the description strings in a way that you can
immediately see what was the test case that failed. If you ever had a
complicated test fail on a random assert somewhere in the middle,
without any description, you may know why this is quite useful :)

If you just wanna see what the output looks like without running it,
see [1], scroll down about halfway.

Lukas

[1] http://blog.coldflake.com/posts/Testing-C++-with-a-new-Catch/


> > ---
> >  src/unittests/test-mjpeg-fallback.cpp | 88
> >  +++++++++++++++++++----------------
> >  1 file changed, 48 insertions(+), 40 deletions(-)
> > 
> > diff --git a/src/unittests/test-mjpeg-fallback.cpp
> > b/src/unittests/test-mjpeg-fallback.cpp
> > index 08ed406..cb2cb68 100644
> > --- a/src/unittests/test-mjpeg-fallback.cpp
> > +++ b/src/unittests/test-mjpeg-fallback.cpp
> > @@ -6,45 +6,53 @@
> >  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'."
> > -        );
> > +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'."
> > +                );
> > +            }
> > +        }
> >      }
> >  }
> 
> Frediano