Re: [PATCH 2/3] testsuite: add support for testing output against regex

[Lucas De Marchi <lucas.de.marchi@gmail.com>]

On Thu, Dec 20, 2018 at 7:00 AM Yauheni Kaliuta
<yauheni.kaliuta@redhat.com> wrote:
>
> Hi, Lucas!
>
> >>>>> On Tue, 18 Dec 2018 16:02:28 -0800, Lucas De Marchi  wrote:
>
>  > Allow to test outputs when they don't match exactly, but should follow
>  > some regex patterns. This can be used when the info we are printing is
>  > randomized or depends on kernel configuration.
>  > ---
>  >  testsuite/testsuite.c | 104 +++++++++++++++++++++++++++++++++++++++++-
>  >  testsuite/testsuite.h |   6 +++
>  >  2 files changed, 108 insertions(+), 2 deletions(-)
>
>  > diff --git a/testsuite/testsuite.c b/testsuite/testsuite.c
>  > index 550c711..db36324 100644
>  > --- a/testsuite/testsuite.c
>  > +++ b/testsuite/testsuite.c
>  > @@ -20,6 +20,7 @@
>  >  #include <fcntl.h>
>  >  #include <getopt.h>
>  >  #include <limits.h>
>  > +#include <regex.h>
>  >  #include <stdarg.h>
>  >  #include <stdio.h>
>  >  #include <stdlib.h>
>  > @@ -293,8 +294,98 @@ static int check_activity(int fd, bool activity,  const char *path,
>  >  #define BUFSZ 4096
>  >  struct buffer {
>  >      char buf[BUFSZ];
>  > +    unsigned int head;
>  >  };
>
>  > +
>  > +static bool cmpbuf_regex_one(const char *pattern, const char *s)
>  > +{
>  > +    _cleanup_(regfree) regex_t re = { };
>  > +
>  > +    return !regcomp(&re, pattern, REG_EXTENDED|REG_NOSUB) &&
>  > +           !regexec(&re, s, 0, NULL, 0);
>  > +}
>  > +
>  > +/*
>  > + * read fd and fd_match, checking the first matches the regex of the second,
>  > + * line by line
>  > + */
>  > +static bool cmpbuf_regex(const struct test *t, const char *prefix,
>  > +                     int fd, int fd_match, struct buffer *buf,
>  > +                     struct buffer *buf_match)
>  > +{
>  > +    char *p, *p_match;
>  > +    int done = 0, done_match = 0, r;
>  > +
>  > +    r = read(fd, buf->buf + buf->head, sizeof(buf->buf) - buf->head - 1);
>
> Why do you need -1 if you are replacing existing '\n' and using
> mem* functions with explicit size?

so buf + head never points outside of the buffer and so avoid a
segfault. It could be rather replace by sanity checks on top of the
function though. I'm changing that.


>
>
>  > +    if (r <= 0)
>  > +            return true;
>  > +
>  > +    buf->head += r;
>  > +
>  > +    /*
>  > +     * Process as many lines as read from fd and that fits in the buffer -
>  > +     * it's assumed that we we get N lines from fd, we should be able to
>                           ^^^^^^^^^ extra "we"?

fixed
>
>  > +     * get the same amount from fd_match
>  > +     */
>  > +    for (;;) {
>  > +            p = memchr(buf->buf + done, '\n', buf->head - done);
>  > +            if (!p)
>  > +                    break;
>  > +            *p = 0;
>
> Would '\0' be better to emphasize the string end?

I use them interchangeably... no preference. I'll change it here though.

>
>  > +
>  > +            p_match = memchr(buf_match->buf + done_match, '\n',
>  > +                             buf_match->head - done_match);
>  > +            if (!p_match) {
>  > +                    /* pump more data from file */
>  > +                    r = read(fd_match, buf_match->buf + buf_match->head,
>  > +                             sizeof(buf_match->buf) - buf_match->head - 1);
>  > +                    if (r <= 0) {
>  > +                            ERR("could not read match fd %d\n", fd_match);
>  > +                            return false;
>  > +                    }
>  > +                    buf_match->head += r;
>  > +                    p_match = memchr(buf_match->buf + done_match, '\n',
>  > +                                     buf_match->head - done_match);
>  > +                    if (!p_match) {
>  > +                            ERR("could not find match line from fd %d\n", fd_match);
>  > +                            return false;
>  > +                    }
>  > +            }
>  > +            *p_match = 0;
>
> ditto.

fixed

Thanks,
Lucas De Marchi

>
>  > +
>  > +            if (!cmpbuf_regex_one(buf_match->buf + done_match, buf->buf + done)) {
>  > +                    ERR("Output does not match pattern on %s:\n", prefix);
>  > +                    ERR("pattern: %s\n", buf_match->buf + done_match);
>  > +                    ERR("output : %s\n", buf->buf + done);
>  > +                    return false;
>  > +            }
>  > +
>  > +            done = p - buf->buf + 1;
>  > +            done_match = p_match - buf_match->buf + 1;
>  > +    }
>  > +
>  > +    /*
>  > +     * Prepare for the next call: anything we processed we remove from the
>  > +     * buffer by memmoving the remaining bytes up to the beginning
>  > +     */
>  > +
>  > +    if (done) {
>  > +            if (buf->head - done)
>  > +                    memmove(buf->buf, buf->buf + done, buf->head - done);
>  > +            buf->head -= done;
>  > +    }
>  > +
>  > +    if (done_match) {
>  > +            if (buf_match->head - done_match)
>  > +                    memmove(buf_match->buf, buf_match->buf + done_match,
>  > +                            buf_match->head - done_match);
>  > +            buf_match->head -= done_match;
>  > +    }
>  > +
>  > +    return true;
>  > +}
>  > +
>  >  /* read fd and fd_match, checking they match exactly */
>  >  static bool cmpbuf_exact(const struct test *t, const char *prefix,
>  >                       int fd, int fd_match, struct buffer *buf,
>  > @@ -428,6 +519,7 @@ static bool test_run_parent_check_outputs(const struct test *t,
>
>  >              for (i = 0;  i < fdcount; i++) {
>  >                      int fd = *(int *)ev[i].data.ptr;
>  > +                    bool ret;
>
>  >                      if (ev[i].events & EPOLLIN) {
>  >                              int fd_match;
>  > @@ -452,9 +544,17 @@ static bool test_run_parent_check_outputs(const struct test *t,
>
>  >                              buf_match = buf + 1;
>
>  > -                            if (!cmpbuf_exact(t, prefix,  fd, fd_match,
>  > -                                              buf, buf_match))
>  > +                            if (t->output.regex)
>  > +                                    ret = cmpbuf_regex(t, prefix, fd, fd_match,
>  > +                                                       buf, buf_match);
>  > +                            else
>  > +                                    ret = cmpbuf_exact(t, prefix,  fd, fd_match,
>  > +                                                       buf, buf_match);
>  > +
>  > +                            if (!ret) {
>  > +                                    err = -1;
>  >                                      goto out;
>  > +                            }
>  >                      } else if (ev[i].events & EPOLLHUP) {
>  >                              if (epoll_ctl(fd_ep, EPOLL_CTL_DEL, fd, NULL) < 0) {
>  >                                      ERR("could not remove fd %d from epoll: %m\n", fd);
>  > diff --git a/testsuite/testsuite.h b/testsuite/testsuite.h
>  > index 2b31483..7ed96bf 100644
>  > --- a/testsuite/testsuite.h
>  > +++ b/testsuite/testsuite.h
>  > @@ -88,6 +88,12 @@ struct test {
>  >              /* File with correct stderr */
>  >              const char *err;
>
>  > +            /*
>  > +             * whether to treat the correct files as regex to the real
>  > +             * output
>  > +             */
>  > +            bool regex;
>  > +
>  >              /*
>  >               * Vector with pair of files
>  >               * key = correct file
>  > --
>  > 2.20.0
>
>
> --
> WBR,
> Yauheni Kaliuta



-- 
Lucas De Marchi