[2/2] ci: enable unit tests under travis-ci

  When building under Travis (or another linux CI service), enable running the
fast-tests for selected builds.  Only the shared builds are enabled at this
point, since they are the ones passing.  Builds that are statically linked
still show some issues in some of the eal_flags tests.  Additionally,
the command to invoke fast tests includes a timeout multiplier, since
some CI environments don't have enough resources to complete the tests in
the default 10s timeout period.

Signed-off-by: Aaron Conole <aconole@redhat.com>
---
 .ci/linux-build.sh | 8 ++++++++
 .ci/linux-setup.sh | 8 +++++++-
 .travis.yml        | 9 ++++++++-
 3 files changed, 23 insertions(+), 2 deletions(-)
  

On Wed, Jul 31, 2019 at 10:50 AM Aaron Conole <aconole@redhat.com> wrote:
>
> When building under Travis (or another linux CI service), enable running the
> fast-tests for selected builds.  Only the shared builds are enabled at this
> point, since they are the ones passing.  Builds that are statically linked
> still show some issues in some of the eal_flags tests.  Additionally,
> the command to invoke fast tests includes a timeout multiplier, since
> some CI environments don't have enough resources to complete the tests in
> the default 10s timeout period.
>
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
>  .ci/linux-build.sh | 8 ++++++++
>  .ci/linux-setup.sh | 8 +++++++-
>  .travis.yml        | 9 ++++++++-
>  3 files changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
> index d5783c1a4..75f740648 100755
> --- a/.ci/linux-build.sh
> +++ b/.ci/linux-build.sh
> @@ -22,3 +22,11 @@ fi
>  OPTS="$OPTS --default-library=$DEF_LIB"
>  meson build --werror -Dexamples=all $OPTS
>  ninja -C build
> +
> +if [ "$RUN_TESTS" = "1" ]; then
> +    # On the test build, also build the documentation, since it's expensive
> +    # and we shouldn't need to build so much of it.
> +    ninja -C build doc
> +
> +    sudo meson test -C build --suite fast-tests -t 3
> +fi
> diff --git a/.ci/linux-setup.sh b/.ci/linux-setup.sh
> index acdf9f370..a40e62eaa 100755
> --- a/.ci/linux-setup.sh
> +++ b/.ci/linux-setup.sh
> @@ -1,3 +1,9 @@
>  #!/bin/sh
>
> -python3 -m pip install --upgrade meson --user
> +# need to install as 'root' since some of the unit tests won't run without it
> +sudo python3 -m pip install --upgrade meson
> +
> +# setup hugepages
> +cat /proc/meminfo
> +sudo sh -c 'echo 1024 > /proc/sys/vm/nr_hugepages'
> +cat /proc/meminfo
Can we drop cat /proc/meminfo?
> diff --git a/.travis.yml b/.travis.yml
> index 7b167fa64..c0c27bb7f 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -30,6 +30,7 @@ env:
>    - DEF_LIB="shared"
>    - DEF_LIB="static" OPTS="-Denable_kmods=false"
>    - DEF_LIB="shared" OPTS="-Denable_kmods=false"
> +  - DEF_LIB="shared" RUN_TESTS=1
I don't agree with this. This is redundant. Why not put RUN_TESTS=1 on
an already exiting builds instead of adding two new builds like you
are doing here?
A build without the tests takes ~7 minutes, with the tests it
increases to ~9 minutes. These two new builds add ~18 minutes of build
time to the entire travis build time. We could use this ~18 minutes
instead to run the tests on 6 to 9 already existing builds since they
take 2 to 3 extra minutes on each one.
>
>  matrix:
>    include:
> @@ -51,7 +52,7 @@ matrix:
>        apt:
>          packages:
>            - *extra_packages
> -  - env: DEF_LIB="shared" EXTRA_PACKAGES=1
> +  - env: DEF_LIB="shared" EXTRA_PACKAGES=1 RUN_TESTS=1
>      compiler: gcc
>      addons:
>        apt:
> @@ -81,6 +82,12 @@ matrix:
>        apt:
>          packages:
>            - *extra_packages
> +  - env: DEF_LIB="shared" EXTRA_PACKAGES=1 RUN_TESTS=1
> +    compiler: clang
> +    addons:
> +      apt:
> +        packages:
> +          - *extra_packages
>    - env: DEF_LIB="static" OPTS="-Denable_kmods=false" EXTRA_PACKAGES=1
>      compiler: clang
>      addons:
> --
> 2.21.0
>
  

Michael Santana Francisco <msantana@redhat.com> writes:

> On Wed, Jul 31, 2019 at 10:50 AM Aaron Conole <aconole@redhat.com> wrote:
>>
>> When building under Travis (or another linux CI service), enable running the
>> fast-tests for selected builds.  Only the shared builds are enabled at this
>> point, since they are the ones passing.  Builds that are statically linked
>> still show some issues in some of the eal_flags tests.  Additionally,
>> the command to invoke fast tests includes a timeout multiplier, since
>> some CI environments don't have enough resources to complete the tests in
>> the default 10s timeout period.
>>
>> Signed-off-by: Aaron Conole <aconole@redhat.com>
>> ---
>>  .ci/linux-build.sh | 8 ++++++++
>>  .ci/linux-setup.sh | 8 +++++++-
>>  .travis.yml        | 9 ++++++++-
>>  3 files changed, 23 insertions(+), 2 deletions(-)
>>
>> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
>> index d5783c1a4..75f740648 100755
>> --- a/.ci/linux-build.sh
>> +++ b/.ci/linux-build.sh
>> @@ -22,3 +22,11 @@ fi
>>  OPTS="$OPTS --default-library=$DEF_LIB"
>>  meson build --werror -Dexamples=all $OPTS
>>  ninja -C build
>> +
>> +if [ "$RUN_TESTS" = "1" ]; then
>> +    # On the test build, also build the documentation, since it's expensive
>> +    # and we shouldn't need to build so much of it.
>> +    ninja -C build doc
>> +
>> +    sudo meson test -C build --suite fast-tests -t 3
>> +fi
>> diff --git a/.ci/linux-setup.sh b/.ci/linux-setup.sh
>> index acdf9f370..a40e62eaa 100755
>> --- a/.ci/linux-setup.sh
>> +++ b/.ci/linux-setup.sh
>> @@ -1,3 +1,9 @@
>>  #!/bin/sh
>>
>> -python3 -m pip install --upgrade meson --user
>> +# need to install as 'root' since some of the unit tests won't run without it
>> +sudo python3 -m pip install --upgrade meson
>> +
>> +# setup hugepages
>> +cat /proc/meminfo
>> +sudo sh -c 'echo 1024 > /proc/sys/vm/nr_hugepages'
>> +cat /proc/meminfo
> Can we drop cat /proc/meminfo?
>> diff --git a/.travis.yml b/.travis.yml
>> index 7b167fa64..c0c27bb7f 100644
>> --- a/.travis.yml
>> +++ b/.travis.yml
>> @@ -30,6 +30,7 @@ env:
>>    - DEF_LIB="shared"
>>    - DEF_LIB="static" OPTS="-Denable_kmods=false"
>>    - DEF_LIB="shared" OPTS="-Denable_kmods=false"
>> +  - DEF_LIB="shared" RUN_TESTS=1
> I don't agree with this. This is redundant. Why not put RUN_TESTS=1 on
> an already exiting builds instead of adding two new builds like you
> are doing here?
> A build without the tests takes ~7 minutes, with the tests it
> increases to ~9 minutes. These two new builds add ~18 minutes of build
> time to the entire travis build time. We could use this ~18 minutes
> instead to run the tests on 6 to 9 already existing builds since they
> take 2 to 3 extra minutes on each one.

Hi Michael,

The ovsrobot was off for a bit due to an internal issue.  Now that it's
back, a new build was generated.  Please have a look.  The time delta is
about 8-12 minutes (because builds are in parallel) for a complete run
(that's with all of the various jobs which make up a build).

Do you think it's unreasonable?

-Aaron
  

On Fri, Aug 2, 2019 at 9:34 AM Aaron Conole <aconole@redhat.com> wrote:
>
> Michael Santana Francisco <msantana@redhat.com> writes:
>
> > On Wed, Jul 31, 2019 at 10:50 AM Aaron Conole <aconole@redhat.com> wrote:
> >>
> >> When building under Travis (or another linux CI service), enable running the
> >> fast-tests for selected builds.  Only the shared builds are enabled at this
> >> point, since they are the ones passing.  Builds that are statically linked
> >> still show some issues in some of the eal_flags tests.  Additionally,
> >> the command to invoke fast tests includes a timeout multiplier, since
> >> some CI environments don't have enough resources to complete the tests in
> >> the default 10s timeout period.
> >>
> >> Signed-off-by: Aaron Conole <aconole@redhat.com>
> >> ---
> >>  .ci/linux-build.sh | 8 ++++++++
> >>  .ci/linux-setup.sh | 8 +++++++-
> >>  .travis.yml        | 9 ++++++++-
> >>  3 files changed, 23 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
> >> index d5783c1a4..75f740648 100755
> >> --- a/.ci/linux-build.sh
> >> +++ b/.ci/linux-build.sh
> >> @@ -22,3 +22,11 @@ fi
> >>  OPTS="$OPTS --default-library=$DEF_LIB"
> >>  meson build --werror -Dexamples=all $OPTS
> >>  ninja -C build
> >> +
> >> +if [ "$RUN_TESTS" = "1" ]; then
> >> +    # On the test build, also build the documentation, since it's expensive
> >> +    # and we shouldn't need to build so much of it.
> >> +    ninja -C build doc
> >> +
> >> +    sudo meson test -C build --suite fast-tests -t 3
> >> +fi
> >> diff --git a/.ci/linux-setup.sh b/.ci/linux-setup.sh
> >> index acdf9f370..a40e62eaa 100755
> >> --- a/.ci/linux-setup.sh
> >> +++ b/.ci/linux-setup.sh
> >> @@ -1,3 +1,9 @@
> >>  #!/bin/sh
> >>
> >> -python3 -m pip install --upgrade meson --user
> >> +# need to install as 'root' since some of the unit tests won't run without it
> >> +sudo python3 -m pip install --upgrade meson
> >> +
> >> +# setup hugepages
> >> +cat /proc/meminfo
> >> +sudo sh -c 'echo 1024 > /proc/sys/vm/nr_hugepages'
> >> +cat /proc/meminfo
> > Can we drop cat /proc/meminfo?
> >> diff --git a/.travis.yml b/.travis.yml
> >> index 7b167fa64..c0c27bb7f 100644
> >> --- a/.travis.yml
> >> +++ b/.travis.yml
> >> @@ -30,6 +30,7 @@ env:
> >>    - DEF_LIB="shared"
> >>    - DEF_LIB="static" OPTS="-Denable_kmods=false"
> >>    - DEF_LIB="shared" OPTS="-Denable_kmods=false"
> >> +  - DEF_LIB="shared" RUN_TESTS=1
> > I don't agree with this. This is redundant. Why not put RUN_TESTS=1 on
> > an already exiting builds instead of adding two new builds like you
> > are doing here?
> > A build without the tests takes ~7 minutes, with the tests it
> > increases to ~9 minutes. These two new builds add ~18 minutes of build
> > time to the entire travis build time. We could use this ~18 minutes
> > instead to run the tests on 6 to 9 already existing builds since they
> > take 2 to 3 extra minutes on each one.
>
> Hi Michael,
>
> The ovsrobot was off for a bit due to an internal issue.  Now that it's
> back, a new build was generated.  Please have a look.  The time delta is
> about 8-12 minutes (because builds are in parallel) for a complete run
> (that's with all of the various jobs which make up a build).
>
> Do you think it's unreasonable?
This is reasonable. I didn't take parallelism into account. This looks
better now

Acked-by: Michael Santana <msantana@redhat.com>
>
> -Aaron
  

31/07/2019 22:54, Michael Santana Francisco:
> On Wed, Jul 31, 2019 at 10:50 AM Aaron Conole <aconole@redhat.com> wrote:
> > --- a/.ci/linux-build.sh
> > +++ b/.ci/linux-build.sh
> > @@ -22,3 +22,11 @@ fi
> >  OPTS="$OPTS --default-library=$DEF_LIB"
> >  meson build --werror -Dexamples=all $OPTS
> >  ninja -C build
> > +
> > +if [ "$RUN_TESTS" = "1" ]; then
> > +    # On the test build, also build the documentation, since it's expensive
> > +    # and we shouldn't need to build so much of it.
> > +    ninja -C build doc

I am not sure to understand the comment.
Do you mean you build the documentation only once,
which is when running tests?
Why it is not a new option similar as RUN_TESTS?

> > --- a/.travis.yml
> > +++ b/.travis.yml
> > @@ -30,6 +30,7 @@ env:
> >    - DEF_LIB="shared"
> >    - DEF_LIB="static" OPTS="-Denable_kmods=false"
> >    - DEF_LIB="shared" OPTS="-Denable_kmods=false"
> > +  - DEF_LIB="shared" RUN_TESTS=1
> I don't agree with this. This is redundant. Why not put RUN_TESTS=1 on
> an already exiting builds instead of adding two new builds like you
> are doing here?

I agree it is a strange logic.
Why not use an existing build to run the tests?
  

Thomas Monjalon <thomas@monjalon.net> writes:

> 31/07/2019 22:54, Michael Santana Francisco:
>> On Wed, Jul 31, 2019 at 10:50 AM Aaron Conole <aconole@redhat.com> wrote:
>> > --- a/.ci/linux-build.sh
>> > +++ b/.ci/linux-build.sh
>> > @@ -22,3 +22,11 @@ fi
>> >  OPTS="$OPTS --default-library=$DEF_LIB"
>> >  meson build --werror -Dexamples=all $OPTS
>> >  ninja -C build
>> > +
>> > +if [ "$RUN_TESTS" = "1" ]; then
>> > +    # On the test build, also build the documentation, since it's expensive
>> > +    # and we shouldn't need to build so much of it.
>> > +    ninja -C build doc
>
> I am not sure to understand the comment.
> Do you mean you build the documentation only once,
> which is when running tests?

Yes.

> Why it is not a new option similar as RUN_TESTS?

I mentioned it at:
http://mails.dpdk.org/archives/dev/2019-July/136635.html also.  Because
it adds build time.

>> > --- a/.travis.yml
>> > +++ b/.travis.yml
>> > @@ -30,6 +30,7 @@ env:
>> >    - DEF_LIB="shared"
>> >    - DEF_LIB="static" OPTS="-Denable_kmods=false"
>> >    - DEF_LIB="shared" OPTS="-Denable_kmods=false"
>> > +  - DEF_LIB="shared" RUN_TESTS=1
>> I don't agree with this. This is redundant. Why not put RUN_TESTS=1 on
>> an already exiting builds instead of adding two new builds like you
>> are doing here?
>
> I agree it is a strange logic.
> Why not use an existing build to run the tests?

The biggest reason is when it fails, it is difficult to know why "at a
glance."  When it does fail due to unit tests, it sometimes takes a
long time to load the logs - so just knowing that the failure is likely
in the unit tests area vs. build is helpful to understand where to start
looking.

It isn't a big deal to merge them, though if you'd prefer it.
  

02/08/2019 22:59, Aaron Conole:
> Thomas Monjalon <thomas@monjalon.net> writes:
> > 31/07/2019 22:54, Michael Santana Francisco:
> >> On Wed, Jul 31, 2019 at 10:50 AM Aaron Conole <aconole@redhat.com> wrote:
> >> > --- a/.ci/linux-build.sh
> >> > +++ b/.ci/linux-build.sh
> >> > @@ -22,3 +22,11 @@ fi
> >> >  OPTS="$OPTS --default-library=$DEF_LIB"
> >> >  meson build --werror -Dexamples=all $OPTS
> >> >  ninja -C build
> >> > +
> >> > +if [ "$RUN_TESTS" = "1" ]; then
> >> > +    # On the test build, also build the documentation, since it's expensive
> >> > +    # and we shouldn't need to build so much of it.
> >> > +    ninja -C build doc
> >
> > I am not sure to understand the comment.
> > Do you mean you build the documentation only once,
> > which is when running tests?
> 
> Yes.
> 
> > Why it is not a new option similar as RUN_TESTS?
> 
> I mentioned it at:
> http://mails.dpdk.org/archives/dev/2019-July/136635.html also.  Because
> it adds build time.

I don't understand.
If you set RUN_TESTS and BUILD_DOCS on the same build,
how is it adding build time?
I'm just suggesting to make explicit that tests and docs
are done in the same run.

> >> > --- a/.travis.yml
> >> > +++ b/.travis.yml
> >> > @@ -30,6 +30,7 @@ env:
> >> >    - DEF_LIB="shared"
> >> >    - DEF_LIB="static" OPTS="-Denable_kmods=false"
> >> >    - DEF_LIB="shared" OPTS="-Denable_kmods=false"
> >> > +  - DEF_LIB="shared" RUN_TESTS=1
> >> I don't agree with this. This is redundant. Why not put RUN_TESTS=1 on
> >> an already exiting builds instead of adding two new builds like you
> >> are doing here?
> >
> > I agree it is a strange logic.
> > Why not use an existing build to run the tests?
> 
> The biggest reason is when it fails, it is difficult to know why "at a
> glance."  When it does fail due to unit tests, it sometimes takes a
> long time to load the logs - so just knowing that the failure is likely
> in the unit tests area vs. build is helpful to understand where to start
> looking.
> 
> It isn't a big deal to merge them, though if you'd prefer it.

It looks to be a good reason.
I'm just sad that we cannot reuse an existing build in another way.
But I guess the infrastructure cache (ccache or other) will be enough.
  

Thomas Monjalon <thomas@monjalon.net> writes:

> 02/08/2019 22:59, Aaron Conole:
>> Thomas Monjalon <thomas@monjalon.net> writes:
>> > 31/07/2019 22:54, Michael Santana Francisco:
>> >> On Wed, Jul 31, 2019 at 10:50 AM Aaron Conole <aconole@redhat.com> wrote:
>> >> > --- a/.ci/linux-build.sh
>> >> > +++ b/.ci/linux-build.sh
>> >> > @@ -22,3 +22,11 @@ fi
>> >> >  OPTS="$OPTS --default-library=$DEF_LIB"
>> >> >  meson build --werror -Dexamples=all $OPTS
>> >> >  ninja -C build
>> >> > +
>> >> > +if [ "$RUN_TESTS" = "1" ]; then
>> >> > +    # On the test build, also build the documentation, since it's expensive
>> >> > +    # and we shouldn't need to build so much of it.
>> >> > +    ninja -C build doc
>> >
>> > I am not sure to understand the comment.
>> > Do you mean you build the documentation only once,
>> > which is when running tests?
>> 
>> Yes.
>> 
>> > Why it is not a new option similar as RUN_TESTS?
>> 
>> I mentioned it at:
>> http://mails.dpdk.org/archives/dev/2019-July/136635.html also.  Because
>> it adds build time.
>
> I don't understand.
> If you set RUN_TESTS and BUILD_DOCS on the same build,
> how is it adding build time?
> I'm just suggesting to make explicit that tests and docs
> are done in the same run.

Sure I'll do that - it's not like I need to mine a new environment
variable from somewhere. :)

>> >> > --- a/.travis.yml
>> >> > +++ b/.travis.yml
>> >> > @@ -30,6 +30,7 @@ env:
>> >> >    - DEF_LIB="shared"
>> >> >    - DEF_LIB="static" OPTS="-Denable_kmods=false"
>> >> >    - DEF_LIB="shared" OPTS="-Denable_kmods=false"
>> >> > +  - DEF_LIB="shared" RUN_TESTS=1
>> >> I don't agree with this. This is redundant. Why not put RUN_TESTS=1 on
>> >> an already exiting builds instead of adding two new builds like you
>> >> are doing here?
>> >
>> > I agree it is a strange logic.
>> > Why not use an existing build to run the tests?
>> 
>> The biggest reason is when it fails, it is difficult to know why "at a
>> glance."  When it does fail due to unit tests, it sometimes takes a
>> long time to load the logs - so just knowing that the failure is likely
>> in the unit tests area vs. build is helpful to understand where to start
>> looking.
>> 
>> It isn't a big deal to merge them, though if you'd prefer it.
>
> It looks to be a good reason.
> I'm just sad that we cannot reuse an existing build in another way.
> But I guess the infrastructure cache (ccache or other) will be enough.