On Sat, Oct 21, 2023 at 08:43:00AM -0700, Simon Glass wrote:
> On Fri, 20 Oct 2023 at 14:53, Tom Rini <tr...@konsulko.com> wrote:
> >
> > The primary motivation for having a sandbox without LTO build in CI is
> > to ensure that we don't have that option break. We now have the ability
> > to run tests of specific options being enabled/disabled, so drop the
> > parts of CI that build and test that configuration specifically and add
> > a build test instead. We still test that "NO_LTO=1" rather than editing
> > the config file works via the ftrace tests.
> >
> > Signed-off-by: Tom Rini <tr...@konsulko.com>
> > ---
> > This creates a small bisectability gap in CI itself, but I think is more
> > reasonable than reworking the introduction of
> > test/py/tests/test_sandbox_opts.py
> >
> > Cc: Simon Glass <s...@chromium.org>
> > ---
> > .azure-pipelines.yml | 3 ---
> > .gitlab-ci.yml | 12 ------------
> > test/py/tests/test_sandbox_opts.py | 10 ++++++++++
> > 3 files changed, 10 insertions(+), 15 deletions(-)
>
> Reviewed-by: Simon Glass <s...@chromium.org>
>
> Q below
>
> >
> > diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml
> > index 6f91553e8613..65533b36dde4 100644
> > --- a/.azure-pipelines.yml
> > +++ b/.azure-pipelines.yml
> > @@ -287,9 +287,6 @@ stages:
> > sandbox64_clang:
> > TEST_PY_BD: "sandbox64"
> > OVERRIDE: "-O clang-16"
> > - sandbox_nolto:
> > - TEST_PY_BD: "sandbox"
> > - BUILD_ENV: "NO_LTO=1"
> > sandbox_spl:
> > TEST_PY_BD: "sandbox_spl"
> > TEST_PY_TEST_SPEC: "test_ofplatdata or test_handoff or test_spl"
> > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> > index 6decdfdee334..97c964fb8079 100644
> > --- a/.gitlab-ci.yml
> > +++ b/.gitlab-ci.yml
> > @@ -258,12 +258,6 @@ sandbox with clang test.py:
> > OVERRIDE: "-O clang-16"
> > <<: *buildman_and_testpy_dfn
> >
> > -sandbox without LTO test.py:
> > - variables:
> > - TEST_PY_BD: "sandbox"
> > - BUILD_ENV: "NO_LTO=1"
> > - <<: *buildman_and_testpy_dfn
> > -
> > sandbox64 test.py:
> > variables:
> > TEST_PY_BD: "sandbox64"
> > @@ -275,12 +269,6 @@ sandbox64 with clang test.py:
> > OVERRIDE: "-O clang-16"
> > <<: *buildman_and_testpy_dfn
> >
> > -sandbox64 without LTO test.py:
> > - variables:
> > - TEST_PY_BD: "sandbox64"
> > - BUILD_ENV: "NO_LTO=1"
> > - <<: *buildman_and_testpy_dfn
> > -
> > sandbox_spl test.py:
> > variables:
> > TEST_PY_BD: "sandbox_spl"
> > diff --git a/test/py/tests/test_sandbox_opts.py
> > b/test/py/tests/test_sandbox_opts.py
> > index 91790b3374b4..422b43cb3bc1 100644
> > --- a/test/py/tests/test_sandbox_opts.py
> > +++ b/test/py/tests/test_sandbox_opts.py
> > @@ -18,3 +18,13 @@ def test_sandbox_cmdline(u_boot_console):
> > out = util.run_and_log(
> > cons, ['./tools/buildman/buildman', '-m', '--board', 'sandbox',
> > '-a', '~CMDLINE', '-o', TMPDIR])
> > +
> > +@pytest.mark.slow
> > +@pytest.mark.boardspec('sandbox')
> > +def test_sandbox_lto(u_boot_console):
> > + """Test building sandbox without CONFIG_LTO"""
> > + cons = u_boot_console
> > +
> > + out = util.run_and_log(
> > + cons, ['./tools/buildman/buildman', '-m', '--board', 'sandbox',
> > + '-a', '~LTO', '-o', TMPDIR])
>
> Don't you need sandbox64 here?No, I don't think it's providing further value to just build sandbox64 without LTO. I'm also not 100% sure this patch is really needed in that we're trying to test for "did we disable LTO via make arguments" and in turn only the ftrace test really catches that, as it will fail if we do have LTO enabled, yes? -- Tom
signature.asc
Description: PGP signature
