| Message ID | 20240612052322.218726-13-npiggin@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | powerpc improvements | expand |
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
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
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
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 --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
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(+)