[PATCH 0/3] RFC: test: Bring in the test hooks

[Tom Rini]

On Fri, May 09, 2025 at 03:52:29PM +0200, Simon Glass wrote:
> Hi Tom,
> 
> On Tue, 6 May 2025 at 21:28, Tom Rini <trini at konsulko.com> wrote:
> >
> > On Tue, May 06, 2025 at 08:10:30PM +0200, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Tue, 6 May 2025 at 18:25, Tom Rini <trini at konsulko.com> wrote:
> > > >
> > > > On Tue, May 06, 2025 at 03:23:59PM +0200, Simon Glass wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Mon, 5 May 2025 at 20:18, Tom Rini <trini at konsulko.com> wrote:
> > > > > >
> > > > > > On Sat, May 03, 2025 at 08:29:13AM -0600, Simon Glass wrote:
> > > > > > > Hi Tom,
> > > > > > >
> > > > > > > On Fri, 2 May 2025 at 14:04, Tom Rini <trini at konsulko.com>
> wrote:
> > > > > > > >
> > > > > > > > On Fri, May 02, 2025 at 09:42:56PM +0200, Heinrich Schuchardt
> wrote:
> > > > > > > > > On 5/2/25 21:30, Tom Rini wrote:
> > > > > > > > > > On Fri, May 02, 2025 at 07:19:18PM +0200, Heinrich
> Schuchardt wrote:
> > > > > > > > > > > On 5/2/25 17:06, Tom Rini wrote:
> > > > > > > > > > > > On Fri, May 02, 2025 at 08:49:12AM -0600, Simon Glass
> wrote:
> > > > > > > > > > > > > Hi Tom,
> > > > > > > > > > > > >
> > > > > > > > > > > > > On Fri, 2 May 2025 at 08:34, Tom Rini <
> trini at konsulko.com> wrote:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > On Thu, May 01, 2025 at 08:50:16PM -0600, Simon
> Glass wrote:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > > During a recent discussion with Heinrich we
> discussed why the hooks are
> > > > > > > > > > > > > > > kept in a separate repo.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > The amount of code is small, a tenth of the
> size of the recently added
> > > > > > > > > > > > > > > lwip, just by way of example. Testing is a
> critical part of U-Boot and
> > > > > > > > > > > > > > > one of the things that distinguishes it from
> firmware projects that have
> > > > > > > > > > > > > > > not kept up in this area. By having the tests
> somewhere else, we are
> > > > > > > > > > > > > > > signalling that it is unusual, or difficult, or
> optional.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > The hooks mechanism also needs something of an
> update to take account of
> > > > > > > > > > > > > > > real boards in 2025. That will be much easier
> to undertake if the code
> > > > > > > > > > > > > > > that test/py talks to is in the same repo.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > This series brings the hook files in as
> first-class citizens of U-Boot.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > If we do go ahead with this, I will send a
> different series which has
> > > > > > > > > > > > > > > separate commits (with correct author) in the
> u-boot-test-hooks repo.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > I think bringing more projects directly in to the
> repository is a bad
> > > > > > > > > > > > > > idea. Your example of lwip isn't applicable
> because it's a read-only
> > > > > > > > > > > > > > subtree that's maintained outside of the project
> (same as the dts
> > > > > > > > > > > > > > subtree). But sure, lets "Say Yes". That said, we
> still need to:
> > > > > > > > > > > > > > - Remove needless examples from the tree.
> > > > > > > > > > > > > > - Not include personal labs directly in the tree.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > That last one is why I really think this is a bad
> idea. The point of
> > > > > > > > > > > > > > having the hooks standalone is so that any given
> lab can easily add
> > > > > > > > > > > > > > support for their lab and manage it, without
> worrying about disclosing
> > > > > > > > > > > > > > internal layout. There's going to be hard coded
> default passwords there.
> > > > > > > > > > >
> > > > > > > > > > > Secrets MUST NEVER reside in git repos.
> > > > > > > > > > >
> > > > > > > > > > > I can't imagine that such irresponsible habits would be
> practiced by any
> > > > > > > > > > > accountable developer.
> > > > > > > > > >
> > > > > > > > > > I agree it shouldn't be done. And just this week was the
> most recent
> > > > > > > > > > time I've seen a company do that internally, because it's
> internal and
> > > > > > > > > > was just a temporary thing. We helped them stop needing
> to do that, but
> > > > > > > > > > assuming it never happens is a bad idea.
> > > > > > > > > >
> > > > > > > > > > > Please, have a look at
> > > > > > > > > > >
> > > > > > > > > > >
> https://docs.github.com/en/actions/security-for-github-actions/security-guides/using-secrets-in-github-actions
> > > > > > > > > > >
> https://tsi-ccdoc.readthedocs.io/en/master/ResOps/2019/gitlab/07_pass-build-secrets.html
> > > > > > > > > > >
> > > > > > > > > > > to see how to do it properly.
> > > > > > > > > > >
> > > > > > > > > > > The same is true for a private lab:
> > > > > > > > > > > Use environment variables that are coming from outside
> the repository.
> > > > > > > > > > >
> > > > > > > > > > > > > > There's going to be repository secrets there.
> That kind of information
> > > > > > > > > > > > > > really should not be in a public repository.
> Integrating the hooks with
> > > > > > > > > > > > > > mainline will make lab management harder, not
> easier. The point of the
> > > > > > > > > > > > > > existing labs in u-boot-test-hooks is to provide
> samples.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > I think this is all why no, we should not go down
> this path.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Is it worth discussing this, or is your mind made
> up? I have some
> > > > > > > > > > > > > thoughts on the last one.
> > > > > > > > > > > >
> > > > > > > > > > > > I think it's a terrible idea that I already said:
> > > > > > > > > > > > > But sure, lets "Say Yes".
> > > > > > > > > > > >
> > > > > > > > > > > > But please do spend time explaining your thoughts and
> perhaps others
> > > > > > > > > > > > will also agree with you and I'll feel less bad
> taking this in?
> > > > > > >
> > > > > > > Well firstly, we should bring in the whole git history so I
> don't
> > > > > > > believe these patches should be applied as is. I did push a
> branch[1]
> > > > > > > but can send the commands if it helps.
> > > > > >
> > > > > > I don't see how that's relevant here, but OK.
> > > > > >
> > > > > > > > > > > Currently when changing a test hook I have to go
> through these steps:
> > > > > > > > > > >
> > > > > > > > > > > * fork u-boot-test-hooks
> > > > > > > > > > > * push a commit to my fork
> > > > > > > > > > > * update both .gitlab-ci.yaml and .azure-pipelines.yaml
> > > > > > > > > > > * commit the change code.denx.de
> > > > > > > > > > > * create a merge request for github.com/u-boot/u-boot
> > > > > > > > > > >
> > > > > > > > > > > If the test hooks were in the same repo as main U-Boot,
> I could save two
> > > > > > > > > > > steps.
> > > > > > > > > >
> > > > > > > > > > Yes, that's true, the steps for updating our CI are a
> little bit longer.
> > > > > > > > > > I would be happy to make u-boot-test-hooks more widely
> writable (so it
> > > > > > > > > > would instead be pushing to a branch for step 1+2). But
> I'm also
> > > > > > > > > > concerned with external labs (which is both the origin of
> these scripts,
> > > > > > > > > > and where they're also used).
> > > > > > > > > >
> > > > > > > > > > Today, I have both a "konsulko" branch of
> u-boot-test-hooks, internally.
> > > > > > > > > > That has subdirectories for both "sage" and "lootbox"
> (the two lab
> > > > > > > > > > hosts), and in turn py and bin subdirectories for each.
> There's no value
> > > > > > > > > > in publishing these changes as there's nothing unique
> about the
> > > > > > > > > > platforms there, which aren't already in the mainline
> u-boot-test-hooks.
> > > > > > > > > >
> > > > > > > > > > How should I manage these changes moving forward?
> > > > > > > > >
> > > > > > > > > You could create two branches based on origin/master u-boot:
> > > > > > > > >
> > > > > > > > > One for maintaining your test lab specific u-boot-test
> changes and one for
> > > > > > > > > your private u-boot code changes.
> > > > > > > > >
> > > > > > > > > Check out the lab specific u-boot-test branch for steering
> the tests of the
> > > > > > > > > u-boot code branch.
> > > > > > > >
> > > > > > > > So now for a lab you need to maintain a branch of U-Boot
> itself, and
> > > > > > > > then to test you need to merge your lab support in? And then
> if there's
> > > > > > > > problems work back what git hash you tested on, if reporting
> either
> > > > > > > > problems, or as part of the "wouldn't it be nice to collect
> results
> > > > > > > > idea" work backwards from there? And bisecting problems then
> becomes a
> > > > > > > > real nightmare since each step requires a merge.
> > > > > > >
> > > > > > > If you have your own hook scripts, they don't have to be in the
> U-Boot
> > > > > > > tree. That is entirely up to you. In my case I would want them
> in the
> > > > > > > tree, but in your case you can put them somewhere else.  You
> can even
> > > > > >
> > > > > > But we shouldn't have more "ellesmere" stuff in the tree. That's a
> > > > > > problem. No ones lab should directly be in upstream. We should be
> > > > > > documenting and providing examples for others to use. Perhaps that
> > > > > > fundamental confusion is why you're suggesting this path?
> > > > >
> > > > > I don't see why ellesmere shouldn't be in there. It is a lab used
> for
> > > > > U-Boot and therefore part of the project, as I see it. If we end up
> > > > > with lots of labs from people then we can worry about that problem
> > > > > then. Today, we have the opposite problem: not enough labs.
> > > >
> > > > You're also unaware of the number of labs we have because today
> there's
> > > > no need for them to post anything. And no, your lab specific
> > > > configuration is not appropriate or useful in the general project.
> This
> > > > is another example of you having a problem seeing the difference
> between
> > > > "this is good for me" and "this is good for the project". Reference
> > > > material should be associated with the project in one way or another,
> > > > not your specific lab.
> > >
> > > That's not quite it. I don't mind seeing other people's labs and in
> > > fact it would help a lot. You ended up sending me an email with lots
> > > of stuff after I had floundered around for hours trying to
> > > reverse-engineer a test. If it had been in-tree I would have solved it
> > > in 5 minutes.
> >
> > Yes, we need better documentation. This has been known for some time.
> > It's somewhere on my TODO list to follow up on the RFC I sent before.
> >
> > > > > > > put them inside the U-Boot directory but without adding them to
> git,
> > > > > > > if that helps. But we set up the 'PATH' variable to where
> things are.
> > > > > > > If we need to change things to make this easier for people, we
> can.
> > > > > > > I'm really not seeing any nightmares here at all.
> > > > > >
> > > > > > I don't see how PATH is relevant here either. That won't pick up
> any of
> > > > > > the configuration files. Might just be able to get away with two
> > > > > > directories in PYTHONPATH instead of one.
> > > > >
> > > > > PATH is enough to run a hook script. Yes you need PYTHONPATH as well
> > > > > if you have stuff in those files. Nothing changes.
> > > > >
> > > > > But Neil's comment was 'So this will definitely remove the ability
> to
> > > > > have test hooks out of the U-boot tree ?', not 'Do I still have to
> set
> > > > > my PYTHONPATH / PATH' to where my tree is ?'.
> > > >
> > > > Yes, I think you don't see how everyone else uses this then. If
> everyone
> > > > else has to set PATH to point to an out of tree copy of
> > > > u-boot-test-hooks which is no longer maintained what has anyone
> gained?
> > >
> > > Perhaps access to some useful examples. But they certainly have not
> > > lost anything.
> > >
> > > >
> > > > I'm open to suggestions on making the workflow Heinrich outlined for
> > > > hooks easier, I'm happy to make the repository writable to other
> > > > custodians, but moving them in-tree makes lots of problems and
> seemingly
> > > > solves few.
> > >
> > > At this point I believe the hooks should be in-tree and I have not
> > > heard any real argument here yet. Neil had the wrong end of the stick
> > > and your argument is that people who don't want their hooks in the
> > > tree won't gained anything by being allowed to put them in the
> > > tree...well, sure.
> > >
> > > If there is any compelling argument, I'd really like to hear it.
> >
> > I think the best compelling argument is that you continue to have the
> > wrong end of the stick yourself as your labgrid implementation bears
> > little resemblance to how the rest of the hooks are implemented and
> > used, based on your comments about how nothing would be lost and there
> > would be no problems.
> 
> There are good reasons for that and I have explained some of them at
> length. I continue to feel that the current test hooks need a refresh too,
> but it is too hard to take that on while the hooks are out-of-tree.
 
> Since I'm not hearing any strong arguments against, I'm planning to pull
> the hooks into my tree, at least. It seems best to preserve the commit
> history, so I'll do that too. It is something like 170 commits.

How about you don't make your personal tree even more different from
mainline? I swear, I really think you should just leave. I am sick and
tired of this.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20250509/6db2e992/attachment.sig>