[12/14] test/eal: make the test pass again

  From: Michael Santana <msantana@redhat.com>

The eal_flags_autotest test currently fails due to a memory leak in the
timer library[1][2]. This failure occurs when the test calls one of its
subtests test_file_prefix().

Fixing the memory leak is not trivial, so this patch is a workaround that
makes the eal_flags_autotest test pass. This is accomplished by moving the
test_file_prefix test to its own test unit. This is a temporary measure
until the leak is fixed.

[1] http://patchwork.dpdk.org/patch/53268/
[2] http://patchwork.dpdk.org/patch/53334/

Signed-off-by: Michael Santana <msantana@redhat.com>
---
 app/test/autotest_data.py | 6 ++++++
 app/test/meson.build      | 1 +
 app/test/test_eal_flags.c | 7 +------
 3 files changed, 8 insertions(+), 6 deletions(-)
  

Hi David and Michael,

David Marchand <david.marchand@redhat.com> writes:

> From: Michael Santana <msantana@redhat.com>
>
> The eal_flags_autotest test currently fails due to a memory leak in the
> timer library[1][2]. This failure occurs when the test calls one of its
> subtests test_file_prefix().
>
> Fixing the memory leak is not trivial, so this patch is a workaround that
> makes the eal_flags_autotest test pass. This is accomplished by moving the
> test_file_prefix test to its own test unit. This is a temporary measure
> until the leak is fixed.
>
> [1] http://patchwork.dpdk.org/patch/53268/
> [2] http://patchwork.dpdk.org/patch/53334/
>
> Signed-off-by: Michael Santana <msantana@redhat.com>
> ---

I'm wondering if it's better to just fix the leak outright.

Then again, it might be useful to have the file-prefix test as an
enumerable test anyway.

CC'd Anatoly.

>  app/test/autotest_data.py | 6 ++++++
>  app/test/meson.build      | 1 +
>  app/test/test_eal_flags.c | 7 +------
>  3 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/app/test/autotest_data.py b/app/test/autotest_data.py
> index 6cf7eca..15e672f 100644
> --- a/app/test/autotest_data.py
> +++ b/app/test/autotest_data.py
> @@ -93,6 +93,12 @@
>          "Report":  None,
>      },
>      {
> +        "Name":    "EAL flags file prefix autotest",
> +        "Command": "eal_flags_prefix_autotest",
> +        "Func":    default_autotest,
> +        "Report":  None,
> +    },
> +    {
>          "Name":    "Hash autotest",
>          "Command": "hash_autotest",
>          "Func":    default_autotest,
> diff --git a/app/test/meson.build b/app/test/meson.build
> index 7ad3684..212cd1b 100644
> --- a/app/test/meson.build
> +++ b/app/test/meson.build
> @@ -156,6 +156,7 @@ fast_parallel_test_names = [
>          'cycles_autotest',
>          'debug_autotest',
>          'eal_flags_autotest',
> +        'eal_flags_prefix_autotest',
>          'eal_fs_autotest',
>          'errno_autotest',
>          'event_ring_autotest',
> diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c
> index cfa8a61..1e227aa 100644
> --- a/app/test/test_eal_flags.c
> +++ b/app/test/test_eal_flags.c
> @@ -1397,12 +1397,6 @@ enum hugepage_action {
>  		return ret;
>  	}
>  
> -	ret = test_file_prefix();
> -	if (ret < 0) {
> -		printf("Error in test_file_prefix()\n");
> -		return ret;
> -	}
> -
>  	ret = test_misc_flags();
>  	if (ret < 0) {
>  		printf("Error in test_misc_flags()");
> @@ -1413,3 +1407,4 @@ enum hugepage_action {
>  }
>  
>  REGISTER_TEST_COMMAND(eal_flags_autotest, test_eal_flags);
> +REGISTER_TEST_COMMAND(eal_flags_prefix_autotest, test_file_prefix);
  

On Tue, Jun 4, 2019 at 3:29 PM Aaron Conole <aconole@redhat.com> wrote:

> Hi David and Michael,
>
> David Marchand <david.marchand@redhat.com> writes:
>
> > From: Michael Santana <msantana@redhat.com>
> >
> > The eal_flags_autotest test currently fails due to a memory leak in the
> > timer library[1][2]. This failure occurs when the test calls one of its
> > subtests test_file_prefix().
> >
> > Fixing the memory leak is not trivial, so this patch is a workaround that
> > makes the eal_flags_autotest test pass. This is accomplished by moving
> the
> > test_file_prefix test to its own test unit. This is a temporary measure
> > until the leak is fixed.
> >
> > [1] http://patchwork.dpdk.org/patch/53268/
> > [2] http://patchwork.dpdk.org/patch/53334/
> >
> > Signed-off-by: Michael Santana <msantana@redhat.com>
> > ---
>
> I'm wondering if it's better to just fix the leak outright.
>
> Then again, it might be useful to have the file-prefix test as an
> enumerable test anyway.
>

We have a lot of tests that embed subtests that can be entirely skipped at
the first subtest failure.
Their execution time can also be quite long making it harder to adjust the
timeout.
So splitting the existing tests into their subtests is something I have
been considering.
  

On 04-Jun-19 9:59 AM, David Marchand wrote:
> From: Michael Santana <msantana@redhat.com>
> 
> The eal_flags_autotest test currently fails due to a memory leak in the
> timer library[1][2]. This failure occurs when the test calls one of its
> subtests test_file_prefix().
> 
> Fixing the memory leak is not trivial, so this patch is a workaround that
> makes the eal_flags_autotest test pass. This is accomplished by moving the
> test_file_prefix test to its own test unit. This is a temporary measure
> until the leak is fixed.
> 
> [1] http://patchwork.dpdk.org/patch/53268/
> [2] http://patchwork.dpdk.org/patch/53334/
> 
> Signed-off-by: Michael Santana <msantana@redhat.com>
> ---

There is now a patchset that fixes the leak [1], so this may not be 
necessary.

[1] https://mails.dpdk.org/archives/dev/2019-June/135774.html
  

On Wed, Jun 26, 2019 at 11:49 AM Burakov, Anatoly <anatoly.burakov@intel.com>
wrote:

> On 04-Jun-19 9:59 AM, David Marchand wrote:
> > From: Michael Santana <msantana@redhat.com>
> >
> > The eal_flags_autotest test currently fails due to a memory leak in the
> > timer library[1][2]. This failure occurs when the test calls one of its
> > subtests test_file_prefix().
> >
> > Fixing the memory leak is not trivial, so this patch is a workaround that
> > makes the eal_flags_autotest test pass. This is accomplished by moving
> the
> > test_file_prefix test to its own test unit. This is a temporary measure
> > until the leak is fixed.
> >
> > [1] http://patchwork.dpdk.org/patch/53268/
> > [2] http://patchwork.dpdk.org/patch/53334/
> >
> > Signed-off-by: Michael Santana <msantana@redhat.com>
> > ---
>
> There is now a patchset that fixes the leak [1], so this may not be
> necessary.
>
> [1] https://mails.dpdk.org/archives/dev/2019-June/135774.html


As discussed on irc, there is a v2 for this series and this patch has been
reworked to be more generic and also fix the CI timeout issues.

I will propagate those previous acks you gave on the v3 if I ever send one.

Thanks.