Re: [PATCH spice-streaming-agent v2 6/9] Add a unit test for the stream port

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

 



On Wed, 2018-05-16 at 12:47 -0400, Frediano Ziglio wrote:
> > 
> > Signed-off-by: Lukáš Hrázký <lhrazky@xxxxxxxxxx>
> > ---
> >  src/unittests/.gitignore           |  1 +
> >  src/unittests/Makefile.am          |  9 ++++-
> >  src/unittests/test-stream-port.cpp | 69
> >  ++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 78 insertions(+), 1 deletion(-)
> >  create mode 100644 src/unittests/test-stream-port.cpp
> > 
> > diff --git a/src/unittests/.gitignore b/src/unittests/.gitignore
> > index 22f1335..ef9e31b 100644
> > --- a/src/unittests/.gitignore
> > +++ b/src/unittests/.gitignore
> > @@ -2,4 +2,5 @@
> >  /test-*.log
> >  /test-*.trs
> >  /test-mjpeg-fallback
> > +/test-stream-port
> >  /test-suite.log
> > diff --git a/src/unittests/Makefile.am b/src/unittests/Makefile.am
> > index 0fc6d50..67f9c42 100644
> > --- a/src/unittests/Makefile.am
> > +++ b/src/unittests/Makefile.am
> > @@ -16,11 +16,12 @@ AM_CFLAGS = \
> >  check_PROGRAMS = \
> >  	hexdump \
> >  	test-mjpeg-fallback \
> > +	test-stream-port \
> >  	$(NULL)
> >  
> >  TESTS = \
> >  	test-hexdump.sh \
> > -	test-mjpeg-fallback \
> > +	test-stream-port \
> >  	$(NULL)
> >  
> 
> Why did you remove test-mjpeg-fallback ?

By accident! I'll put it back, don't know how that happened.

> >  noinst_PROGRAMS = \
> > @@ -46,6 +47,12 @@ test_mjpeg_fallback_LDADD = \
> >  	$(JPEG_LIBS) \
> >  	$(NULL)
> >  
> > +test_stream_port_SOURCES = \
> > +	test-stream-port.cpp \
> > +	../stream-port.cpp \
> > +	../error.cpp \
> > +	$(NULL)
> > +
> >  EXTRA_DIST = \
> >  	test-hexdump.sh \
> >  	hexdump1.in \
> > diff --git a/src/unittests/test-stream-port.cpp
> > b/src/unittests/test-stream-port.cpp
> > new file mode 100644
> > index 0000000..3f9dadf
> > --- /dev/null
> > +++ b/src/unittests/test-stream-port.cpp
> > @@ -0,0 +1,69 @@
> 
> Didn't notice previously.
> You should add the copyright/file header.

Yeah, I forgot it here, will add it.

> > +#define CATCH_CONFIG_MAIN
> > +#include <catch/catch.hpp>
> > +#include <sys/socket.h>
> > +
> > +#include "stream-port.hpp"
> > +
> > +
> > +namespace ssa = spice::streaming_agent;
> > +
> > +/*
> > + * Note the semantics of a socketpair may be different from the virtio port
> > + * that is actually used for the real interface.
> > + */
> > +SCENARIO("test basic IO on the stream port", "[port][io]") {
> > +    GIVEN("An open port (socketpair)") {
> > +        int fd[2];
> > +        const char *src_buf = "brekeke";
> > +        const size_t src_size = strlen(src_buf);
> > +
> > +        socketpair(AF_LOCAL, SOCK_STREAM, 0, fd);
> > +
> > +        WHEN("reading data in one go") {
> > +            CHECK(write(fd[0], src_buf, src_size) == src_size);
> > +            char buf[10];
> > +            ssa::read_all(fd[1], buf, src_size);
> > +            CHECK(std::string(buf, src_size) == src_buf);
> > +        }
> > +
> > +        WHEN("reading data in two steps") {
> > +            CHECK(write(fd[0], src_buf, src_size) == src_size);
> > +            char buf[10];
> > +            ssa::read_all(fd[1], buf, 3);
> > +            CHECK(std::string(buf, 3) == "bre");
> > +            ssa::read_all(fd[1], buf, 4);
> > +            CHECK(std::string(buf, 4) == "keke");
> > +        }
> > +
> > +        WHEN("writing data") {
> > +            ssa::write_all(fd[1], src_buf, src_size);
> > +            char buf[10];
> > +            CHECK(read(fd[0], buf, src_size) == src_size);
> > +            CHECK(std::string(buf, src_size) == src_buf);
> > +        }
> > +
> > +        WHEN("closing the remote end and trying to read") {
> > +            CHECK(write(fd[0], src_buf, src_size) == src_size);
> > +            char buf[10];
> > +            ssa::read_all(fd[1], buf, 3);
> > +            CHECK(std::string(buf, 3) == "bre");
> > +            CHECK(close(fd[0]) == 0);
> > +            ssa::read_all(fd[1], buf, 4);
> > +            CHECK(std::string(buf, 4) == "keke");
> > +            // TODO blocks infinitely, we should recognize the remote end is
> > closed
> > +            //ssa::read_all(fd[1], buf, 1);
> > +        }
> > +
> > +        WHEN("closing the remote end and trying to write") {
> > +            ssa::write_all(fd[1], src_buf, src_size);
> > +            char buf[10];
> > +            CHECK(close(fd[0]) == 0);
> > +            // TODO causes a SIGPIPE
> > +            //ssa::write_all(fd[1], src_buf, src_size);
> > +        }
> 
> This is testing nothing actually, I would add the complete scenario
> on following patch

Well, it documents the failing test case, if it wasn't for the
discrepancy in behaviour of the virtio port and the socketpair, it
could also be used to investigate the bug (before it was fixed) easily
in the test case. But if you insist, I'll remove it.

Cheers,
Lukas

> > +
> > +        // clean up the descriptors in case they are still open
> > +        close(fd[0]);
> > +        close(fd[1]);
> > +    }
> > +}
> 
> Frediano
_______________________________________________
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]