diff mbox series

[v3] oeqa/runtime: fix race-condition in minidebuginfo test

Message ID 20240610161642.257051-1-ecordonnier@snap.com
State New
Headers show
Series [v3] oeqa/runtime: fix race-condition in minidebuginfo test | expand

Commit Message

Etienne Cordonnier June 10, 2024, 4:16 p.m. UTC
From: Etienne Cordonnier <ecordonnier@snap.com>

Fix this error where 'coredumpctl info' warns that the coredump is still being
processed:

```
AssertionError: 1 != 0 : MiniDebugInfo Test failed: No match found.
-- Notice: 1 systemd-coredump@.service unit is running, output may be incomplete.
```

Signed-off-by: Etienne Cordonnier <ecordonnier@snap.com>
---
 meta/lib/oeqa/runtime/cases/systemd.py | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Ross Burton June 11, 2024, 10:47 a.m. UTC | #1
On 10 Jun 2024, at 17:16, Etienne Cordonnier via lists.openembedded.org <ecordonnier=snap.com@lists.openembedded.org> wrote:
> 
> From: Etienne Cordonnier <ecordonnier@snap.com>
> 
> Fix this error where 'coredumpctl info' warns that the coredump is still being
> processed:
> 
> ```
> AssertionError: 1 != 0 : MiniDebugInfo Test failed: No match found.
> -- Notice: 1 systemd-coredump@.service unit is running, output may be incomplete.
> ```
> 
> Signed-off-by: Etienne Cordonnier <ecordonnier@snap.com>
> ---
> meta/lib/oeqa/runtime/cases/systemd.py | 7 +++++++
> 1 file changed, 7 insertions(+)
> 
> diff --git a/meta/lib/oeqa/runtime/cases/systemd.py b/meta/lib/oeqa/runtime/cases/systemd.py
> index 80fdae240a..aed230708e 100644
> --- a/meta/lib/oeqa/runtime/cases/systemd.py
> +++ b/meta/lib/oeqa/runtime/cases/systemd.py
> @@ -155,6 +155,13 @@ class SystemdServiceTests(SystemdTest):
>         self.target.run('kill -SEGV %s' % output)
>         self.assertEqual(status, 0, msg = 'Not able to find process that runs sleep, output : %s' % output)
> 
> +        # Give some time to systemd-coredump@.service to process the coredump
> +        for x in range(20):
> +            status, output = self.target.run('coredumpctl --quiet --no-pager --no-legend | wc -l')
> +            if output == "1":
> +                break
> +            time.sleep(1)
> +
>         (status, output) = self.target.run('coredumpctl info')

As someone who spent too long staring at mysterious failures in the debuginfod tests, I recommend you add an ‘else’ case to that loop so that you can explicitly fail if the timeout expires without breaking out.

Also it feels like the test could be made more reliably by using matches instead of just asking it to list all crashes and expecting it to be one (what if something else crashed?), and assuming the latest crash is the one we’re after.   If you use matches then the exit code is set to non-zero if the match isn’t found, so you could combine the retry loop and the actual test into one.

Ross
Etienne Cordonnier June 11, 2024, 1:34 p.m. UTC | #2
Hi Ross,
OK, I addressed your feedback. On a side note, coredumpctl matches based on
process names are really confusing when used with symlinks. I used a
match based on pid to avoid the confusion:
"
# coredumpctl list
TIME                         PID UID GID SIG     COREFILE EXE
   SIZE
Tue 2024-05-21 05:36:50 GMT 3442   0   0 SIGSEGV present
 /usr/bin/coreutils 109.0K

# coredumpctl list coreutils
No coredumps found.

# coredumpctl list sleep
TIME                         PID UID GID SIG     COREFILE EXE
   SIZE
Tue 2024-05-21 05:36:50 GMT 3442   0   0 SIGSEGV present
 /usr/bin/coreutils 109.0K
"


On Tue, Jun 11, 2024 at 12:47 PM Ross Burton <Ross.Burton@arm.com> wrote:

> On 10 Jun 2024, at 17:16, Etienne Cordonnier via lists.openembedded.org
> <ecordonnier=snap.com@lists.openembedded.org> wrote:
> >
> > From: Etienne Cordonnier <ecordonnier@snap.com>
> >
> > Fix this error where 'coredumpctl info' warns that the coredump is still
> being
> > processed:
> >
> > ```
> > AssertionError: 1 != 0 : MiniDebugInfo Test failed: No match found.
> > -- Notice: 1 systemd-coredump@.service unit is running, output may be
> incomplete.
> > ```
> >
> > Signed-off-by: Etienne Cordonnier <ecordonnier@snap.com>
> > ---
> > meta/lib/oeqa/runtime/cases/systemd.py | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/meta/lib/oeqa/runtime/cases/systemd.py
> b/meta/lib/oeqa/runtime/cases/systemd.py
> > index 80fdae240a..aed230708e 100644
> > --- a/meta/lib/oeqa/runtime/cases/systemd.py
> > +++ b/meta/lib/oeqa/runtime/cases/systemd.py
> > @@ -155,6 +155,13 @@ class SystemdServiceTests(SystemdTest):
> >         self.target.run('kill -SEGV %s' % output)
> >         self.assertEqual(status, 0, msg = 'Not able to find process that
> runs sleep, output : %s' % output)
> >
> > +        # Give some time to systemd-coredump@.service to process the
> coredump
> > +        for x in range(20):
> > +            status, output = self.target.run('coredumpctl --quiet
> --no-pager --no-legend | wc -l')
> > +            if output == "1":
> > +                break
> > +            time.sleep(1)
> > +
> >         (status, output) = self.target.run('coredumpctl info')
>
> As someone who spent too long staring at mysterious failures in the
> debuginfod tests, I recommend you add an ‘else’ case to that loop so that
> you can explicitly fail if the timeout expires without breaking out.
>
> Also it feels like the test could be made more reliably by using matches
> instead of just asking it to list all crashes and expecting it to be one
> (what if something else crashed?), and assuming the latest crash is the one
> we’re after.   If you use matches then the exit code is set to non-zero if
> the match isn’t found, so you could combine the retry loop and the actual
> test into one.
>
> Ross
diff mbox series

Patch

diff --git a/meta/lib/oeqa/runtime/cases/systemd.py b/meta/lib/oeqa/runtime/cases/systemd.py
index 80fdae240a..aed230708e 100644
--- a/meta/lib/oeqa/runtime/cases/systemd.py
+++ b/meta/lib/oeqa/runtime/cases/systemd.py
@@ -155,6 +155,13 @@  class SystemdServiceTests(SystemdTest):
         self.target.run('kill -SEGV %s' % output)
         self.assertEqual(status, 0, msg = 'Not able to find process that runs sleep, output : %s' % output)
 
+        # Give some time to systemd-coredump@.service to process the coredump
+        for x in range(20):
+            status, output = self.target.run('coredumpctl --quiet --no-pager --no-legend | wc -l')
+            if output == "1":
+                break
+            time.sleep(1)
+
         (status, output) = self.target.run('coredumpctl info')
         self.assertEqual(status, 0, msg='MiniDebugInfo Test failed: %s' % output)
         self.assertEqual('sleep_for_duration (busybox.nosuid' in output or 'xnanosleep (sleep.coreutils' in output,