* [PATCH v2] bitbake: runqueue: Verify mcdepends are valid
@ 2025-02-25 18:38 Mark Hatle
2025-02-27 12:29 ` [bitbake-devel] " Richard Purdie
0 siblings, 1 reply; 3+ messages in thread
From: Mark Hatle @ 2025-02-25 18:38 UTC (permalink / raw)
To: bitbake-devel
From: Mark Hatle <mark.hatle@amd.com>
In order to avoid a potentially confusing backtrace, check that the mcdepend
is valid when we add it.
Add a test case to ensure invalid configurations are caught and trigger an
error.
Signed-off-by: Mark Hatle <mark.hatle@amd.com>
Signed-off-by: Mark Hatle <mark.hatle@kernel.crashing.org>
---
lib/bb/runqueue.py | 2 ++
lib/bb/tests/runqueue-tests/recipes/g1.bb | 2 ++
lib/bb/tests/runqueue-tests/recipes/h1.bb | 0
lib/bb/tests/runqueue.py | 18 ++++++++++++++++++
4 files changed, 22 insertions(+)
create mode 100644 lib/bb/tests/runqueue-tests/recipes/g1.bb
create mode 100644 lib/bb/tests/runqueue-tests/recipes/h1.bb
diff --git a/lib/bb/runqueue.py b/lib/bb/runqueue.py
index ffb2d2849..539a6065f 100644
--- a/lib/bb/runqueue.py
+++ b/lib/bb/runqueue.py
@@ -730,6 +730,8 @@ class RunQueueData:
if mc == frommc:
fn = taskData[mcdep].build_targets[pn][0]
newdep = '%s:%s' % (fn,deptask)
+ if newdep not in taskData[mcdep].taskentries:
+ bb.fatal("Task mcdepends on non-existent task %s" % (newdep))
taskData[mc].taskentries[tid].tdepends.append(newdep)
for mc in taskData:
diff --git a/lib/bb/tests/runqueue-tests/recipes/g1.bb b/lib/bb/tests/runqueue-tests/recipes/g1.bb
new file mode 100644
index 000000000..3c7dca025
--- /dev/null
+++ b/lib/bb/tests/runqueue-tests/recipes/g1.bb
@@ -0,0 +1,2 @@
+do_build[mcdepends] = "mc::mc-1:h1:do_invalid"
+
diff --git a/lib/bb/tests/runqueue-tests/recipes/h1.bb b/lib/bb/tests/runqueue-tests/recipes/h1.bb
new file mode 100644
index 000000000..e69de29bb
diff --git a/lib/bb/tests/runqueue.py b/lib/bb/tests/runqueue.py
index cc87e8d6a..e15fdc330 100644
--- a/lib/bb/tests/runqueue.py
+++ b/lib/bb/tests/runqueue.py
@@ -314,6 +314,24 @@ class RunQueueTests(unittest.TestCase):
["mc_2:a1:%s" % t for t in rerun_tasks]
self.assertEqual(set(tasks), set(expected))
+ # AssertionError is raised by self.run_bitbakecmd in the event of a failure
+ # in thise case a failure is required, but we need to do further processing
+ # to tell if it's the RIGHT kind of failure.
+ with self.assertRaises(AssertionError):
+ try:
+ self.run_bitbakecmd(["bitbake", "g1"], tempdir, "", extraenv=extraenv, cleanup=True)
+ except AssertionError as e:
+ # If the word 'Traceback' or 'KeyError' is in the exception text,
+ # we've regressed. So verify it's NOT present, and pass the
+ # exception to indicate a 'pass'.
+ if not ('Traceback' in str(e) or 'KeyError' in str(e)):
+ # Raising 'AssertionError' is a test pass
+ raise e
+ else:
+ # Dump the output for triage, but don't raise an
+ # exception, this indicates a test failure.
+ print("%s: %s" % (type(e), e))
+
self.shutdown(tempdir)
def test_hashserv_single(self):
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [bitbake-devel] [PATCH v2] bitbake: runqueue: Verify mcdepends are valid
2025-02-25 18:38 [PATCH v2] bitbake: runqueue: Verify mcdepends are valid Mark Hatle
@ 2025-02-27 12:29 ` Richard Purdie
2025-02-27 17:04 ` Mark Hatle
0 siblings, 1 reply; 3+ messages in thread
From: Richard Purdie @ 2025-02-27 12:29 UTC (permalink / raw)
To: mark.hatle, bitbake-devel; +Cc: Ross Burton
On Tue, 2025-02-25 at 12:38 -0600, Mark Hatle via lists.openembedded.org wrote:
> From: Mark Hatle <mark.hatle@amd.com>
>
> In order to avoid a potentially confusing backtrace, check that the mcdepend
> is valid when we add it.
>
> Add a test case to ensure invalid configurations are caught and trigger an
> error.
>
> Signed-off-by: Mark Hatle <mark.hatle@amd.com>
> Signed-off-by: Mark Hatle <mark.hatle@kernel.crashing.org>
> ---
> lib/bb/runqueue.py | 2 ++
> lib/bb/tests/runqueue-tests/recipes/g1.bb | 2 ++
> lib/bb/tests/runqueue-tests/recipes/h1.bb | 0
> lib/bb/tests/runqueue.py | 18 ++++++++++++++++++
> 4 files changed, 22 insertions(+)
> create mode 100644 lib/bb/tests/runqueue-tests/recipes/g1.bb
> create mode 100644 lib/bb/tests/runqueue-tests/recipes/h1.bb
>
> diff --git a/lib/bb/runqueue.py b/lib/bb/runqueue.py
> index ffb2d2849..539a6065f 100644
> --- a/lib/bb/runqueue.py
> +++ b/lib/bb/runqueue.py
> @@ -730,6 +730,8 @@ class RunQueueData:
> if mc == frommc:
> fn = taskData[mcdep].build_targets[pn][0]
> newdep = '%s:%s' % (fn,deptask)
> + if newdep not in taskData[mcdep].taskentries:
> + bb.fatal("Task mcdepends on non-existent task %s" % (newdep))
> taskData[mc].taskentries[tid].tdepends.append(newdep)
>
> for mc in taskData:
> diff --git a/lib/bb/tests/runqueue-tests/recipes/g1.bb b/lib/bb/tests/runqueue-tests/recipes/g1.bb
> new file mode 100644
> index 000000000..3c7dca025
> --- /dev/null
> +++ b/lib/bb/tests/runqueue-tests/recipes/g1.bb
> @@ -0,0 +1,2 @@
> +do_build[mcdepends] = "mc::mc-1:h1:do_invalid"
> +
> diff --git a/lib/bb/tests/runqueue-tests/recipes/h1.bb b/lib/bb/tests/runqueue-tests/recipes/h1.bb
> new file mode 100644
> index 000000000..e69de29bb
> diff --git a/lib/bb/tests/runqueue.py b/lib/bb/tests/runqueue.py
> index cc87e8d6a..e15fdc330 100644
> --- a/lib/bb/tests/runqueue.py
> +++ b/lib/bb/tests/runqueue.py
> @@ -314,6 +314,24 @@ class RunQueueTests(unittest.TestCase):
> ["mc_2:a1:%s" % t for t in rerun_tasks]
> self.assertEqual(set(tasks), set(expected))
>
> + # AssertionError is raised by self.run_bitbakecmd in the event of a failure
> + # in thise case a failure is required, but we need to do further processing
> + # to tell if it's the RIGHT kind of failure.
> + with self.assertRaises(AssertionError):
> + try:
> + self.run_bitbakecmd(["bitbake", "g1"], tempdir, "", extraenv=extraenv, cleanup=True)
> + except AssertionError as e:
> + # If the word 'Traceback' or 'KeyError' is in the exception text,
> + # we've regressed. So verify it's NOT present, and pass the
> + # exception to indicate a 'pass'.
> + if not ('Traceback' in str(e) or 'KeyError' in str(e)):
> + # Raising 'AssertionError' is a test pass
> + raise e
> + else:
> + # Dump the output for triage, but don't raise an
> + # exception, this indicates a test failure.
> + print("%s: %s" % (type(e), e))
> +
> self.shutdown(tempdir)
>
> def test_hashserv_single(self):
>
Sorry to ask this but during review Ross pointed out that
assertRaisesRegex can take a regex and then you can really simplify
this further. Would you be able to tweak this further to use that?
As I've mentioned elsewhere, having well written tests helps foster
further well written tests in the future. Given how long we end up
maintaining these for, it is worth getting them right!
Cheers,
Richard
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bitbake-devel] [PATCH v2] bitbake: runqueue: Verify mcdepends are valid
2025-02-27 12:29 ` [bitbake-devel] " Richard Purdie
@ 2025-02-27 17:04 ` Mark Hatle
0 siblings, 0 replies; 3+ messages in thread
From: Mark Hatle @ 2025-02-27 17:04 UTC (permalink / raw)
To: bitbake-devel
On 2/27/25 6:29 AM, Richard Purdie via lists.openembedded.org wrote:
> On Tue, 2025-02-25 at 12:38 -0600, Mark Hatle via lists.openembedded.org wrote:
>> From: Mark Hatle <mark.hatle@amd.com>
>>
>> In order to avoid a potentially confusing backtrace, check that the mcdepend
>> is valid when we add it.
>>
>> Add a test case to ensure invalid configurations are caught and trigger an
>> error.
>>
>> Signed-off-by: Mark Hatle <mark.hatle@amd.com>
>> Signed-off-by: Mark Hatle <mark.hatle@kernel.crashing.org>
>> ---
>> lib/bb/runqueue.py | 2 ++
>> lib/bb/tests/runqueue-tests/recipes/g1.bb | 2 ++
>> lib/bb/tests/runqueue-tests/recipes/h1.bb | 0
>> lib/bb/tests/runqueue.py | 18 ++++++++++++++++++
>> 4 files changed, 22 insertions(+)
>> create mode 100644 lib/bb/tests/runqueue-tests/recipes/g1.bb
>> create mode 100644 lib/bb/tests/runqueue-tests/recipes/h1.bb
>>
>> diff --git a/lib/bb/runqueue.py b/lib/bb/runqueue.py
>> index ffb2d2849..539a6065f 100644
>> --- a/lib/bb/runqueue.py
>> +++ b/lib/bb/runqueue.py
>> @@ -730,6 +730,8 @@ class RunQueueData:
>> if mc == frommc:
>> fn = taskData[mcdep].build_targets[pn][0]
>> newdep = '%s:%s' % (fn,deptask)
>> + if newdep not in taskData[mcdep].taskentries:
>> + bb.fatal("Task mcdepends on non-existent task %s" % (newdep))
>> taskData[mc].taskentries[tid].tdepends.append(newdep)
>>
>> for mc in taskData:
>> diff --git a/lib/bb/tests/runqueue-tests/recipes/g1.bb b/lib/bb/tests/runqueue-tests/recipes/g1.bb
>> new file mode 100644
>> index 000000000..3c7dca025
>> --- /dev/null
>> +++ b/lib/bb/tests/runqueue-tests/recipes/g1.bb
>> @@ -0,0 +1,2 @@
>> +do_build[mcdepends] = "mc::mc-1:h1:do_invalid"
>> +
>> diff --git a/lib/bb/tests/runqueue-tests/recipes/h1.bb b/lib/bb/tests/runqueue-tests/recipes/h1.bb
>> new file mode 100644
>> index 000000000..e69de29bb
>> diff --git a/lib/bb/tests/runqueue.py b/lib/bb/tests/runqueue.py
>> index cc87e8d6a..e15fdc330 100644
>> --- a/lib/bb/tests/runqueue.py
>> +++ b/lib/bb/tests/runqueue.py
>> @@ -314,6 +314,24 @@ class RunQueueTests(unittest.TestCase):
>> ["mc_2:a1:%s" % t for t in rerun_tasks]
>> self.assertEqual(set(tasks), set(expected))
>>
>> + # AssertionError is raised by self.run_bitbakecmd in the event of a failure
>> + # in thise case a failure is required, but we need to do further processing
>> + # to tell if it's the RIGHT kind of failure.
>> + with self.assertRaises(AssertionError):
>> + try:
>> + self.run_bitbakecmd(["bitbake", "g1"], tempdir, "", extraenv=extraenv, cleanup=True)
>> + except AssertionError as e:
>> + # If the word 'Traceback' or 'KeyError' is in the exception text,
>> + # we've regressed. So verify it's NOT present, and pass the
>> + # exception to indicate a 'pass'.
>> + if not ('Traceback' in str(e) or 'KeyError' in str(e)):
>> + # Raising 'AssertionError' is a test pass
>> + raise e
>> + else:
>> + # Dump the output for triage, but don't raise an
>> + # exception, this indicates a test failure.
>> + print("%s: %s" % (type(e), e))
>> +
>> self.shutdown(tempdir)
>>
>> def test_hashserv_single(self):
>>
>
> Sorry to ask this but during review Ross pointed out that
> assertRaisesRegex can take a regex and then you can really simplify
> this further. Would you be able to tweak this further to use that?
I've not been able to get this to work with assertRaisesRegex. What I tried was:
with self.assertRaisesRegex(AssertionError, "(?!(Traceback|KeyError))'):
self.run_bitbakecmd(["bitbake", "g1"], tempdir, "", extraenv=extraenv,
cleanup=True)
With the first hunk of the change disabled, it doesn't fail, even though the
exception text DOES include both 'Traceback' and 'KeyError'. I'm wondering if
this could be a 'multiline' issues, as the traceback looks like
Command ['bitbake', 'g1'] failed with NOTE: Reconnecting to bitbake server...
Note:Retrying server connection (#1)... (09:54:45.246200)
Loading cache...done.
Loaded 28 entries from dependency cache.
NOTE: Resolving any missing task queue dependencies
NOTE: Resolving any missing task queue dependencies
NOTE: Resolving any missing task queue dependencies
NOTE: Resolving any missing task queue dependencies
NOTE: Resolving any missing task queue dependencies
NOTE: Resolving any missing task queue dependencies
NOTE: Resolving any missing task queue dependencies
NOTE: Resolving any missing task queue dependencies
NOTE: Resolving any missing task queue dependencies
Initialising tasks...ERROR: An uncaught exception occurred in runqueue
Traceback (most recent call last):
File "<path>/bitbake/lib/bb/runqueue.py", line 1659, in execute_runqueue
return self._execute_runqueue()
File "<path>/bitbake/lib/bb/runqueue.py", line 1570, in _execute_runqueue
if self.rqdata.prepare() == 0:
File "<path>/bitbake/lib/bb/runqueue.py", line 858, in prepare
revdeps[dep].add(tid)
KeyError:
'mc:mc-1:<path>/bitbake/lib/bb/tests/runqueue-tests/recipes/h1.bb:do_invalid'
(and some more text below)...
Any suggestions on a regex from anyone that can work with the above would be
useful, otherwise we may have hit a limitation if the re.search (which I assume)
that assertRaisesRegex is using.
--Mark
> As I've mentioned elsewhere, having well written tests helps foster
> further well written tests in the future. Given how long we end up
> maintaining these for, it is worth getting them right!
>
> Cheers,
>
> Richard
>
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#17345): https://lists.openembedded.org/g/bitbake-devel/message/17345
> Mute This Topic: https://lists.openembedded.org/mt/111382531/3616948
> Group Owner: bitbake-devel+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/bitbake-devel/unsub [mark.hatle@kernel.crashing.org]
> -=-=-=-=-=-=-=-=-=-=-=-
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-02-27 17:04 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-25 18:38 [PATCH v2] bitbake: runqueue: Verify mcdepends are valid Mark Hatle
2025-02-27 12:29 ` [bitbake-devel] " Richard Purdie
2025-02-27 17:04 ` Mark Hatle
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.