Re: [PATCH 2/2] tests/avocado/boot_linux.py: re-enable test-case for ppc64

2023-03-31 Thread Cédric Le Goater

Hello,

[ Copying qemu-ppc@ and Daniel ]

On 3/28/23 13:24, Kautuk Consul wrote:

On 2023-03-27 17:07:30, Alex Bennée wrote:


Kautuk Consul  writes:


Fixes c0c8687ef0("tests/avocado: disable BootLinuxPPC64 test in CI").

Commit c0c8687ef0fd990db8db1655a8a6c5a5e35dd4bb disabled the test-case
for PPC64. On investigation, this turns out to be an issue with the
time taken for downloading the Fedora 31 qcow2 image being included
within the test-case timeout.
Re-enable this test-case by setting the timeout to 360 seconds just
before launching the downloaded VM image.

Signed-off-by: Kautuk Consul 
Reported-by: Alex Bennée 
Tested-by: Hariharan T S hariharan...@linux.vnet.ibm.com


It doesn't really address the principle problem that the
boot_linux.py:BootLinuxPPC64.test_pseries_tcg is super heavyweight for
only 2% extra coverage of the executed lines.

By re-enabling this test-case we will ensure that PPC64 part of qemu
works okay in terms of basic linux boot. Without this we will have
a regression in the sense that there won't be any way to test out
basic linux boot for PPC64.


There are ways and pseries is not only PPC64 machine. There is more
to it. See :

  https://github.com/legoater/qemu-ppc-boot/tree/main/buildroot
  https://github.com/legoater/buildroot/tree/qemu-ppc/board/qemu

QEMU PPC maintainers have external tools for regressions which are
run regularly, at least before sending a PR for upstream.

Thanks,

C.



What we really need is a script so we can compare the output between the
two jsons:

   gcovr --json --exclude-unreachable-branches --print-summary -o coverage.json 
--root ../../ . *.p

because I suspect we could make up that missing few % noodling the
baseline test a bit more.

Can you tell me how you check code coverage with and without this
test-case ? I am kind of new to qemu so it would be nice to know how you
do this. And I am trying to increase the code coverage by improving
the baseline test by including more devices in the qemu-system-ppc64
command line so I would appreciate any tips on how to do that also.



---
  tests/avocado/boot_linux.py | 6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tests/avocado/boot_linux.py b/tests/avocado/boot_linux.py
index be30dcbd58..c3869a987c 100644
--- a/tests/avocado/boot_linux.py
+++ b/tests/avocado/boot_linux.py
@@ -91,9 +91,9 @@ class BootLinuxPPC64(LinuxTest):
  :avocado: tags=arch:ppc64
  """
  
+# timeout for downloading new VM image.

  timeout = 360
  
-@skipIf(os.getenv('GITLAB_CI'), 'Running on GitLab')

  def test_pseries_tcg(self):
  """
  :avocado: tags=machine:pseries
@@ -101,6 +101,10 @@ def test_pseries_tcg(self):
  """
  self.require_accelerator("tcg")
  self.vm.add_args("-accel", "tcg")
+
+# timeout for actual Linux PPC boot test
+self.timeout = 360
+
  self.launch_and_wait(set_up_ssh_connection=False)



--
Alex Bennée
Virtualisation Tech Lead @ Linaro








Re: [PATCH 2/2] tests/avocado/boot_linux.py: re-enable test-case for ppc64

2023-03-28 Thread Alex Bennée


Kautuk Consul  writes:

> On 2023-03-27 17:07:30, Alex Bennée wrote:
>> 
>> Kautuk Consul  writes:
>> 
>> > Fixes c0c8687ef0("tests/avocado: disable BootLinuxPPC64 test in CI").
>> >
>> > Commit c0c8687ef0fd990db8db1655a8a6c5a5e35dd4bb disabled the test-case
>> > for PPC64. On investigation, this turns out to be an issue with the
>> > time taken for downloading the Fedora 31 qcow2 image being included
>> > within the test-case timeout.
>> > Re-enable this test-case by setting the timeout to 360 seconds just
>> > before launching the downloaded VM image.
>> >
>> > Signed-off-by: Kautuk Consul 
>> > Reported-by: Alex Bennée 
>> > Tested-by: Hariharan T S hariharan...@linux.vnet.ibm.com
>> 
>> It doesn't really address the principle problem that the
>> boot_linux.py:BootLinuxPPC64.test_pseries_tcg is super heavyweight for
>> only 2% extra coverage of the executed lines.
> By re-enabling this test-case we will ensure that PPC64 part of qemu
> works okay in terms of basic linux boot. Without this we will have
> a regression in the sense that there won't be any way to test out
> basic linux boot for PPC64.

Sure we do:

  ➜  ./tests/venv/bin/avocado list 
./tests/avocado/tuxrun_baselines.py:TuxRunBaselineTest.test_p
  INSTRUMENTED ./tests/avocado/tuxrun_baselines.py:TuxRunBaselineTest.test_ppc32
  INSTRUMENTED ./tests/avocado/tuxrun_baselines.py:TuxRunBaselineTest.test_ppc64
  INSTRUMENTED 
./tests/avocado/tuxrun_baselines.py:TuxRunBaselineTest.test_ppc64le

boot 3 different ppc configurations.

>> 
>> What we really need is a script so we can compare the output between the
>> two jsons:
>> 
>>   gcovr --json --exclude-unreachable-branches --print-summary -o 
>> coverage.json --root ../../ . *.p
>> 
>> because I suspect we could make up that missing few % noodling the
>> baseline test a bit more.
> Can you tell me how you check code coverage with and without this
> test-case ?

I use two build directories, both configured with --enable-gcov. e.g.:

 ../../configure' '--disable-docs' '--enable-gcov' '--target-list=ppc64-softmmu'

and run a different set of tests in each build dir. You can then run:

  make coverage-html V=1

for the initial report. See:

  https://qemu.readthedocs.io/en/latest/devel/testing.html#gcc-gcov-support
  
> I am kind of new to qemu so it would be nice to know how you
> do this. And I am trying to increase the code coverage by improving
> the baseline test by including more devices in the qemu-system-ppc64
> command line so I would appreciate any tips on how to do that also.

The only problem is eyeballing the html reports is a very fuzzy way of
comparing coverage. However the gcovr report generates some useful
machine readable json which could be compared with a script.

>> 
>> > ---
>> >  tests/avocado/boot_linux.py | 6 +-
>> >  1 file changed, 5 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/tests/avocado/boot_linux.py b/tests/avocado/boot_linux.py
>> > index be30dcbd58..c3869a987c 100644
>> > --- a/tests/avocado/boot_linux.py
>> > +++ b/tests/avocado/boot_linux.py
>> > @@ -91,9 +91,9 @@ class BootLinuxPPC64(LinuxTest):
>> >  :avocado: tags=arch:ppc64
>> >  """
>> >  
>> > +# timeout for downloading new VM image.
>> >  timeout = 360
>> >  
>> > -@skipIf(os.getenv('GITLAB_CI'), 'Running on GitLab')
>> >  def test_pseries_tcg(self):
>> >  """
>> >  :avocado: tags=machine:pseries
>> > @@ -101,6 +101,10 @@ def test_pseries_tcg(self):
>> >  """
>> >  self.require_accelerator("tcg")
>> >  self.vm.add_args("-accel", "tcg")
>> > +
>> > +# timeout for actual Linux PPC boot test
>> > +self.timeout = 360
>> > +
>> >  self.launch_and_wait(set_up_ssh_connection=False)
>> 
>> 
>> -- 
>> Alex Bennée
>> Virtualisation Tech Lead @ Linaro
>> 


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro



Re: [PATCH 2/2] tests/avocado/boot_linux.py: re-enable test-case for ppc64

2023-03-28 Thread Kautuk Consul
On 2023-03-27 17:07:30, Alex Bennée wrote:
> 
> Kautuk Consul  writes:
> 
> > Fixes c0c8687ef0("tests/avocado: disable BootLinuxPPC64 test in CI").
> >
> > Commit c0c8687ef0fd990db8db1655a8a6c5a5e35dd4bb disabled the test-case
> > for PPC64. On investigation, this turns out to be an issue with the
> > time taken for downloading the Fedora 31 qcow2 image being included
> > within the test-case timeout.
> > Re-enable this test-case by setting the timeout to 360 seconds just
> > before launching the downloaded VM image.
> >
> > Signed-off-by: Kautuk Consul 
> > Reported-by: Alex Bennée 
> > Tested-by: Hariharan T S hariharan...@linux.vnet.ibm.com
> 
> It doesn't really address the principle problem that the
> boot_linux.py:BootLinuxPPC64.test_pseries_tcg is super heavyweight for
> only 2% extra coverage of the executed lines.
By re-enabling this test-case we will ensure that PPC64 part of qemu
works okay in terms of basic linux boot. Without this we will have
a regression in the sense that there won't be any way to test out
basic linux boot for PPC64.
> 
> What we really need is a script so we can compare the output between the
> two jsons:
> 
>   gcovr --json --exclude-unreachable-branches --print-summary -o 
> coverage.json --root ../../ . *.p
> 
> because I suspect we could make up that missing few % noodling the
> baseline test a bit more.
Can you tell me how you check code coverage with and without this
test-case ? I am kind of new to qemu so it would be nice to know how you
do this. And I am trying to increase the code coverage by improving
the baseline test by including more devices in the qemu-system-ppc64
command line so I would appreciate any tips on how to do that also.
> 
> > ---
> >  tests/avocado/boot_linux.py | 6 +-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/tests/avocado/boot_linux.py b/tests/avocado/boot_linux.py
> > index be30dcbd58..c3869a987c 100644
> > --- a/tests/avocado/boot_linux.py
> > +++ b/tests/avocado/boot_linux.py
> > @@ -91,9 +91,9 @@ class BootLinuxPPC64(LinuxTest):
> >  :avocado: tags=arch:ppc64
> >  """
> >  
> > +# timeout for downloading new VM image.
> >  timeout = 360
> >  
> > -@skipIf(os.getenv('GITLAB_CI'), 'Running on GitLab')
> >  def test_pseries_tcg(self):
> >  """
> >  :avocado: tags=machine:pseries
> > @@ -101,6 +101,10 @@ def test_pseries_tcg(self):
> >  """
> >  self.require_accelerator("tcg")
> >  self.vm.add_args("-accel", "tcg")
> > +
> > +# timeout for actual Linux PPC boot test
> > +self.timeout = 360
> > +
> >  self.launch_and_wait(set_up_ssh_connection=False)
> 
> 
> -- 
> Alex Bennée
> Virtualisation Tech Lead @ Linaro
> 



Re: [PATCH 2/2] tests/avocado/boot_linux.py: re-enable test-case for ppc64

2023-03-27 Thread Alex Bennée


Kautuk Consul  writes:

> Fixes c0c8687ef0("tests/avocado: disable BootLinuxPPC64 test in CI").
>
> Commit c0c8687ef0fd990db8db1655a8a6c5a5e35dd4bb disabled the test-case
> for PPC64. On investigation, this turns out to be an issue with the
> time taken for downloading the Fedora 31 qcow2 image being included
> within the test-case timeout.
> Re-enable this test-case by setting the timeout to 360 seconds just
> before launching the downloaded VM image.
>
> Signed-off-by: Kautuk Consul 
> Reported-by: Alex Bennée 
> Tested-by: Hariharan T S hariharan...@linux.vnet.ibm.com

It doesn't really address the principle problem that the
boot_linux.py:BootLinuxPPC64.test_pseries_tcg is super heavyweight for
only 2% extra coverage of the executed lines.

What we really need is a script so we can compare the output between the
two jsons:

  gcovr --json --exclude-unreachable-branches --print-summary -o coverage.json 
--root ../../ . *.p

because I suspect we could make up that missing few % noodling the
baseline test a bit more.

> ---
>  tests/avocado/boot_linux.py | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/tests/avocado/boot_linux.py b/tests/avocado/boot_linux.py
> index be30dcbd58..c3869a987c 100644
> --- a/tests/avocado/boot_linux.py
> +++ b/tests/avocado/boot_linux.py
> @@ -91,9 +91,9 @@ class BootLinuxPPC64(LinuxTest):
>  :avocado: tags=arch:ppc64
>  """
>  
> +# timeout for downloading new VM image.
>  timeout = 360
>  
> -@skipIf(os.getenv('GITLAB_CI'), 'Running on GitLab')
>  def test_pseries_tcg(self):
>  """
>  :avocado: tags=machine:pseries
> @@ -101,6 +101,10 @@ def test_pseries_tcg(self):
>  """
>  self.require_accelerator("tcg")
>  self.vm.add_args("-accel", "tcg")
> +
> +# timeout for actual Linux PPC boot test
> +self.timeout = 360
> +
>  self.launch_and_wait(set_up_ssh_connection=False)


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro



[PATCH 2/2] tests/avocado/boot_linux.py: re-enable test-case for ppc64

2023-03-27 Thread Kautuk Consul
Fixes c0c8687ef0("tests/avocado: disable BootLinuxPPC64 test in CI").

Commit c0c8687ef0fd990db8db1655a8a6c5a5e35dd4bb disabled the test-case
for PPC64. On investigation, this turns out to be an issue with the
time taken for downloading the Fedora 31 qcow2 image being included
within the test-case timeout.
Re-enable this test-case by setting the timeout to 360 seconds just
before launching the downloaded VM image.

Signed-off-by: Kautuk Consul 
Reported-by: Alex Bennée 
Tested-by: Hariharan T S hariharan...@linux.vnet.ibm.com
---
 tests/avocado/boot_linux.py | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tests/avocado/boot_linux.py b/tests/avocado/boot_linux.py
index be30dcbd58..c3869a987c 100644
--- a/tests/avocado/boot_linux.py
+++ b/tests/avocado/boot_linux.py
@@ -91,9 +91,9 @@ class BootLinuxPPC64(LinuxTest):
 :avocado: tags=arch:ppc64
 """
 
+# timeout for downloading new VM image.
 timeout = 360
 
-@skipIf(os.getenv('GITLAB_CI'), 'Running on GitLab')
 def test_pseries_tcg(self):
 """
 :avocado: tags=machine:pseries
@@ -101,6 +101,10 @@ def test_pseries_tcg(self):
 """
 self.require_accelerator("tcg")
 self.vm.add_args("-accel", "tcg")
+
+# timeout for actual Linux PPC boot test
+self.timeout = 360
+
 self.launch_and_wait(set_up_ssh_connection=False)
 
 
-- 
2.39.2