diff mbox series

[kvm-unit-tests,v10,12/15] scripts/arch-run.bash: Fix run_panic() success exit status

Message ID 20240612052322.218726-13-npiggin@gmail.com (mailing list archive)
State Handled Elsewhere, archived
Headers show
Series powerpc improvements | expand

Commit Message

Nicholas Piggin June 12, 2024, 5:23 a.m. UTC 
run_qemu_status() looks for "EXIT: STATUS=%d" if the harness command
returned 1, to determine the final status of the test. In the case of
panic tests, QEMU should terminate before successful exit status is
known, so the run_panic() command must produce the "EXIT: STATUS" line.

With this change, running a panic test returns 0 on success (panic),
and the run_test.sh unit test correctly displays it as PASS rather than
FAIL.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 scripts/arch-run.bash | 1 +
 1 file changed, 1 insertion(+)

Comments

Andrew Jones June 12, 2024, 7:26 a.m. UTC | #1 
On Wed, Jun 12, 2024 at 03:23:17PM GMT, Nicholas Piggin wrote:
> run_qemu_status() looks for "EXIT: STATUS=%d" if the harness command
> returned 1, to determine the final status of the test. In the case of
> panic tests, QEMU should terminate before successful exit status is
> known, so the run_panic() command must produce the "EXIT: STATUS" line.
> 
> With this change, running a panic test returns 0 on success (panic),
> and the run_test.sh unit test correctly displays it as PASS rather than
> FAIL.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  scripts/arch-run.bash | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> index 8643bab3b..9bf2f0bbd 100644
> --- a/scripts/arch-run.bash
> +++ b/scripts/arch-run.bash
> @@ -378,6 +378,7 @@ run_panic ()
>  	else
>  		# some QEMU versions report multiple panic events
>  		echo "PASS: guest panicked"
> +		echo "EXIT: STATUS=1"
>  		ret=1
>  	fi
>  
> -- 
> 2.45.1
>

Do we also need an 'echo "EXIT: STATUS=3"' in the if-arm of this if-else?

Thanks,
drew
Nicholas Piggin June 14, 2024, 12:56 a.m. UTC | #2 
On Wed Jun 12, 2024 at 5:26 PM AEST, Andrew Jones wrote:
> On Wed, Jun 12, 2024 at 03:23:17PM GMT, Nicholas Piggin wrote:
> > run_qemu_status() looks for "EXIT: STATUS=%d" if the harness command
> > returned 1, to determine the final status of the test. In the case of
> > panic tests, QEMU should terminate before successful exit status is
> > known, so the run_panic() command must produce the "EXIT: STATUS" line.
> > 
> > With this change, running a panic test returns 0 on success (panic),
> > and the run_test.sh unit test correctly displays it as PASS rather than
> > FAIL.
> > 
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >  scripts/arch-run.bash | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> > index 8643bab3b..9bf2f0bbd 100644
> > --- a/scripts/arch-run.bash
> > +++ b/scripts/arch-run.bash
> > @@ -378,6 +378,7 @@ run_panic ()
> >  	else
> >  		# some QEMU versions report multiple panic events
> >  		echo "PASS: guest panicked"
> > +		echo "EXIT: STATUS=1"
> >  		ret=1
> >  	fi
> >  
> > -- 
> > 2.45.1
> >
>
> Do we also need an 'echo "EXIT: STATUS=3"' in the if-arm of this if-else?

In that case we don't need it because run_panic() returns 3 in that
case. run_qemu_status() only checks guest status if harness said
QEMU ran successfully.

Arguably this is a bit hacky because "EXIT: STATUS" should really be
a guest-printed status, and we would have a different success code
for expected-QEMU-crash type of tests where guest can't provide status.
I can do that incrementally after this if you like, but it's a bit
more code.

Thanks,
Nick
Andrew Jones June 17, 2024, 2:12 p.m. UTC | #3 
On Fri, Jun 14, 2024 at 10:56:02AM GMT, Nicholas Piggin wrote:
> On Wed Jun 12, 2024 at 5:26 PM AEST, Andrew Jones wrote:
> > On Wed, Jun 12, 2024 at 03:23:17PM GMT, Nicholas Piggin wrote:
> > > run_qemu_status() looks for "EXIT: STATUS=%d" if the harness command
> > > returned 1, to determine the final status of the test. In the case of
> > > panic tests, QEMU should terminate before successful exit status is
> > > known, so the run_panic() command must produce the "EXIT: STATUS" line.
> > > 
> > > With this change, running a panic test returns 0 on success (panic),
> > > and the run_test.sh unit test correctly displays it as PASS rather than
> > > FAIL.
> > > 
> > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > > ---
> > >  scripts/arch-run.bash | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> > > index 8643bab3b..9bf2f0bbd 100644
> > > --- a/scripts/arch-run.bash
> > > +++ b/scripts/arch-run.bash
> > > @@ -378,6 +378,7 @@ run_panic ()
> > >  	else
> > >  		# some QEMU versions report multiple panic events
> > >  		echo "PASS: guest panicked"
> > > +		echo "EXIT: STATUS=1"
> > >  		ret=1
> > >  	fi
> > >  
> > > -- 
> > > 2.45.1
> > >
> >
> > Do we also need an 'echo "EXIT: STATUS=3"' in the if-arm of this if-else?
> 
> In that case we don't need it because run_panic() returns 3 in that
> case. run_qemu_status() only checks guest status if harness said
> QEMU ran successfully.

Ah, yes.

> 
> Arguably this is a bit hacky because "EXIT: STATUS" should really be
> a guest-printed status, and we would have a different success code
> for expected-QEMU-crash type of tests where guest can't provide status.
> I can do that incrementally after this if you like, but it's a bit
> more code.

Just this patch for now works for me.

Thanks,
drew
Andrew Jones June 17, 2024, 2:13 p.m. UTC | #4 
On Wed, Jun 12, 2024 at 03:23:17PM GMT, Nicholas Piggin wrote:
> run_qemu_status() looks for "EXIT: STATUS=%d" if the harness command
> returned 1, to determine the final status of the test. In the case of
> panic tests, QEMU should terminate before successful exit status is
> known, so the run_panic() command must produce the "EXIT: STATUS" line.
> 
> With this change, running a panic test returns 0 on success (panic),
> and the run_test.sh unit test correctly displays it as PASS rather than
> FAIL.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  scripts/arch-run.bash | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> index 8643bab3b..9bf2f0bbd 100644
> --- a/scripts/arch-run.bash
> +++ b/scripts/arch-run.bash
> @@ -378,6 +378,7 @@ run_panic ()
>  	else
>  		# some QEMU versions report multiple panic events
>  		echo "PASS: guest panicked"
> +		echo "EXIT: STATUS=1"
>  		ret=1
>  	fi
>  
> -- 
> 2.45.1
>

Acked-by: Andrew Jones <andrew.jones@linux.dev>
diff mbox series

Patch

diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index 8643bab3b..9bf2f0bbd 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -378,6 +378,7 @@  run_panic ()
 	else
 		# some QEMU versions report multiple panic events
 		echo "PASS: guest panicked"
+		echo "EXIT: STATUS=1"
 		ret=1
 	fi