| Message ID | 20251224141659.2218381-2-uvv.mail@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v2,1/4] overlayfs: Fix QA failure | expand |
On Wed, 2025-12-24 at 14:16 +0000, Vyacheslav Yurkov via lists.openembedded.org wrote: > From: Vyacheslav Yurkov <uvv.mail@gmail.com> > > The test orignally was written under assumption that poky distro is > used. When poky-altcft is used for example, then systemd is already set > in DISTRO_FETURES, which the test did not expect. The commit title says this is about determinism, but it's actually about supporting init systems other than systemd in the test case. I also think "selftest" should come before the component name, so perhaps it should be: selftest: overlayfs: Support non-systemd init systems > > Signed-off-by: Vyacheslav Yurkov <uvv.mail@gmail.com> > --- > meta/lib/oeqa/selftest/cases/overlayfs.py | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/meta/lib/oeqa/selftest/cases/overlayfs.py b/meta/lib/oeqa/selftest/cases/overlayfs.py > index 580fbdcb9c..3e55e97927 100644 > --- a/meta/lib/oeqa/selftest/cases/overlayfs.py > +++ b/meta/lib/oeqa/selftest/cases/overlayfs.py > @@ -5,7 +5,7 @@ > # > > from oeqa.selftest.case import OESelftestTestCase > -from oeqa.utils.commands import bitbake, runqemu, get_bb_vars > +from oeqa.utils.commands import bitbake, runqemu, get_bb_vars, get_bb_var > from oeqa.core.decorator import OETestTag > from oeqa.core.decorator.data import skipIfNotMachine > > @@ -46,7 +46,8 @@ inherit overlayfs > res = bitbake('core-image-minimal', ignore_status=True) > line = getline(res, "overlayfs-user was skipped: missing required distro features") > self.assertTrue("overlayfs" in res.output, msg=res.output) > - self.assertTrue("systemd" in res.output, msg=res.output) > + if not "systemd" in get_bb_var('DISTRO_FEATURES'): > + self.assertTrue("systemd" in res.output, msg=res.output) The inverse logic here confused a few people on the patch review call. Could we add a comment at the same time, something like: # If systemd is not enabled, we should see it listed as a missing # distro feature That is just re-stating what the code does, but I think it's worthwhile here to avoid confusion. Best regards,
On 08.01.2026 14:33, Paul Barker wrote: > On Wed, 2025-12-24 at 14:16 +0000, Vyacheslav Yurkov via > lists.openembedded.org wrote: >> From: Vyacheslav Yurkov <uvv.mail@gmail.com> >> >> The test orignally was written under assumption that poky distro is >> used. When poky-altcft is used for example, then systemd is already set >> in DISTRO_FETURES, which the test did not expect. > The commit title says this is about determinism, but it's actually about > supporting init systems other than systemd in the test case. I also > think "selftest" should come before the component name, so perhaps it > should be: > > selftest: overlayfs: Support non-systemd init systems The commit has nothing to do with non-systemd init systems, because the whole feature only works with systemd. > >> Signed-off-by: Vyacheslav Yurkov <uvv.mail@gmail.com> >> --- >> meta/lib/oeqa/selftest/cases/overlayfs.py | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/meta/lib/oeqa/selftest/cases/overlayfs.py b/meta/lib/oeqa/selftest/cases/overlayfs.py >> index 580fbdcb9c..3e55e97927 100644 >> --- a/meta/lib/oeqa/selftest/cases/overlayfs.py >> +++ b/meta/lib/oeqa/selftest/cases/overlayfs.py >> @@ -5,7 +5,7 @@ >> # >> >> from oeqa.selftest.case import OESelftestTestCase >> -from oeqa.utils.commands import bitbake, runqemu, get_bb_vars >> +from oeqa.utils.commands import bitbake, runqemu, get_bb_vars, get_bb_var >> from oeqa.core.decorator import OETestTag >> from oeqa.core.decorator.data import skipIfNotMachine >> >> @@ -46,7 +46,8 @@ inherit overlayfs >> res = bitbake('core-image-minimal', ignore_status=True) >> line = getline(res, "overlayfs-user was skipped: missing required distro features") >> self.assertTrue("overlayfs" in res.output, msg=res.output) >> - self.assertTrue("systemd" in res.output, msg=res.output) >> + if not "systemd" in get_bb_var('DISTRO_FEATURES'): >> + self.assertTrue("systemd" in res.output, msg=res.output) > The inverse logic here confused a few people on the patch review call. > Could we add a comment at the same time, something like: > > # If systemd is not enabled, we should see it listed as a missing > # distro feature > > That is just re-stating what the code does, but I think it's worthwhile > here to avoid confusion. This is already written a few lines above in the test description. > > Best regards, > Thanks, Slava
Just want to double check, what is the reason the patch was dropped from master-next? Is that only the component order in the caption? Slava On 09.01.2026 09:49, Vyacheslav Yurkov via lists.openembedded.org wrote: > On 08.01.2026 14:33, Paul Barker wrote: >> On Wed, 2025-12-24 at 14:16 +0000, Vyacheslav Yurkov via >> lists.openembedded.org wrote: >>> From: Vyacheslav Yurkov <uvv.mail@gmail.com> >>> >>> The test orignally was written under assumption that poky distro is >>> used. When poky-altcft is used for example, then systemd is already set >>> in DISTRO_FETURES, which the test did not expect. >> The commit title says this is about determinism, but it's actually about >> supporting init systems other than systemd in the test case. I also >> think "selftest" should come before the component name, so perhaps it >> should be: >> >> selftest: overlayfs: Support non-systemd init systems > > The commit has nothing to do with non-systemd init systems, because > the whole feature only works with systemd. > >> >>> Signed-off-by: Vyacheslav Yurkov <uvv.mail@gmail.com> >>> --- >>> meta/lib/oeqa/selftest/cases/overlayfs.py | 5 +++-- >>> 1 file changed, 3 insertions(+), 2 deletions(-) >>> >>> diff --git a/meta/lib/oeqa/selftest/cases/overlayfs.py >>> b/meta/lib/oeqa/selftest/cases/overlayfs.py >>> index 580fbdcb9c..3e55e97927 100644 >>> --- a/meta/lib/oeqa/selftest/cases/overlayfs.py >>> +++ b/meta/lib/oeqa/selftest/cases/overlayfs.py >>> @@ -5,7 +5,7 @@ >>> # >>> from oeqa.selftest.case import OESelftestTestCase >>> -from oeqa.utils.commands import bitbake, runqemu, get_bb_vars >>> +from oeqa.utils.commands import bitbake, runqemu, get_bb_vars, >>> get_bb_var >>> from oeqa.core.decorator import OETestTag >>> from oeqa.core.decorator.data import skipIfNotMachine >>> @@ -46,7 +46,8 @@ inherit overlayfs >>> res = bitbake('core-image-minimal', ignore_status=True) >>> line = getline(res, "overlayfs-user was skipped: missing >>> required distro features") >>> self.assertTrue("overlayfs" in res.output, msg=res.output) >>> - self.assertTrue("systemd" in res.output, msg=res.output) >>> + if not "systemd" in get_bb_var('DISTRO_FEATURES'): >>> + self.assertTrue("systemd" in res.output, msg=res.output) >> The inverse logic here confused a few people on the patch review call. >> Could we add a comment at the same time, something like: >> >> # If systemd is not enabled, we should see it listed as a missing >> # distro feature >> >> That is just re-stating what the code does, but I think it's worthwhile >> here to avoid confusion. > > This is already written a few lines above in the test description. >
diff --git a/meta/lib/oeqa/selftest/cases/overlayfs.py b/meta/lib/oeqa/selftest/cases/overlayfs.py index 580fbdcb9c..3e55e97927 100644 --- a/meta/lib/oeqa/selftest/cases/overlayfs.py +++ b/meta/lib/oeqa/selftest/cases/overlayfs.py @@ -5,7 +5,7 @@ # from oeqa.selftest.case import OESelftestTestCase -from oeqa.utils.commands import bitbake, runqemu, get_bb_vars +from oeqa.utils.commands import bitbake, runqemu, get_bb_vars, get_bb_var from oeqa.core.decorator import OETestTag from oeqa.core.decorator.data import skipIfNotMachine @@ -46,7 +46,8 @@ inherit overlayfs res = bitbake('core-image-minimal', ignore_status=True) line = getline(res, "overlayfs-user was skipped: missing required distro features") self.assertTrue("overlayfs" in res.output, msg=res.output) - self.assertTrue("systemd" in res.output, msg=res.output) + if not "systemd" in get_bb_var('DISTRO_FEATURES'): + self.assertTrue("systemd" in res.output, msg=res.output) self.assertTrue("ERROR: Required build target 'core-image-minimal' has no buildable providers." in res.output, msg=res.output) def test_not_all_units_installed(self):