Hi Tom,

On Wed, 2 Apr 2025 at 05:49, Tom Rini <tr...@konsulko.com> wrote:
>
> On Wed, Apr 02, 2025 at 04:49:18AM +1300, Simon Glass wrote:
> > Hi Tom,
> >
> > On Mon, 31 Mar 2025 at 03:45, Tom Rini <tr...@konsulko.com> wrote:
> > >
> > > On Thu, Mar 20, 2025 at 03:41:33AM +0000, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Wed, 19 Mar 2025 at 16:35, Tom Rini <tr...@konsulko.com> wrote:
> > > > >
> > > > > On Wed, Mar 19, 2025 at 03:04:28PM +0000, Simon Glass wrote:
> > > > > > Hi Tom,
> > > > > >
> > > > > > On Wed, 19 Mar 2025 at 15:52, Tom Rini <tr...@konsulko.com> wrote:
> > > > > > >
> > > > > > > On Wed, Mar 19, 2025 at 03:38:03PM +0100, Simon Glass wrote:
> > > > > > >
> > > > > > > > Add an extlinux image that contains a few Ubuntu entries.
> > > > > > > >
> > > > > > > > Increase the number of sandbox-USB-hub ports to permit this.
> > > > > > > >
> > > > > > > > Signed-off-by: Simon Glass <s...@chromium.org>
> > > > > > >
> > > > > > > I don't understand what this test adds. In neither the current 
> > > > > > > Fedora
> > > > > > > test nor in this new test are we actually booting something, 
> > > > > > > we're just
> > > > > > > taking a sample extlinux.conf and making sure it doesn't fail. Is 
> > > > > > > it
> > > > > > > that we're testing in a useful fashion now having two labels?
> > > > > >
> > > > > > I didn't think so either, which is why I never did this before. But 
> > > > > > it
> > > > > > turns out that there were some bugs, too.
> > > > >
> > > > > I don't understand you, sorry. You don't think so either to what?
> > > >
> > > > I didn't think we needed two extlinux examples on different boot
> > > > devices, but we do.
> > >
> > > I'm not sure it's particularly clear what you're doing here then, or
> > > why, but I'll find some time to read it all more deeply.
> >
> > OK. This test case is how I found the bugs/problems in bootstd that
> > are fixed in this series.
>
> It should be (a) better explained and likely (b) instead of dropping in
> a seemingly verbatim installed file a specially crafted bit of content.
> We aren't testing "Ubuntu". We are testing multiple labels.
>
> > > > > > > We should probably be clear about what we're doing in the tests 
> > > > > > > and
> > > > > > > instead of adding seemingly arbitrary distributions add an 
> > > > > > > extlinux test
> > > > > > > and testcases.
> > > > > >
> > > > > > This is not actually a test case. It is simply creating a new image.
> > > > > > The test cases are in the other patches, so please take a look 
> > > > > > there.
> > > > >
> > > > > Nothing in this series quickly reads as adding tests and fixing 
> > > > > problems
> > > > > with extlinux parsing, it's all bootmeth stuff?
> > > >
> > > > It isn't about the actual parsing of the .conf file, although I would
> > > > like to add tests there as we have none apart from what I have added
> > > > in my PXE series. It's more about having multiple devices with
> > > > bootable OSes on them. This series tidies up and fixes this. We need
> > > > to have an image available on more than one device to spot these
> > > > problems.
> > >
> > > And the existing tests for pxelinux that we have in mainline already,
> > > don't forget those.
> >
> > Yes. But those tests actually don't use bootstd, do they?
>
> No, they're testing pxelinux, the thing you said we don't have any tests
> for.
>
> > > > Currently we have two accessible to sandbox, one extlinux and one
> > > > EFI*. I decided to add a third, using extlinux.
> > > >
> > > > Again, this is not a test case, but provides an image for the test
> > > > cases in this series.
> > >
> > > Adding mocked up things for use in sandbox is adding test cases.
> >
> > One is a test image for use by tests; the other is a test. Perhaps you
> > are just saying that there is no point in having one without the
> > other? Otherwise, I'm not sure what to do with this feedback.
>
> Maybe I clarified better above now. You're not testing Ubuntu (or Fedora
> or Armbian or ...) you're seeing if various pxelinux files parse
> correctly. Making the tests be clearer about what's being tested is
> what's missing at least in part.

You want me to remove the word 'Ubuntu' from the test files? This is
what I get when I install u-boot-tools so I am trying to use that,
rather than invent my own thing. This is the same approach I've taken
with Fedora and Armbian and I think it makes the most sense.

Regards,
Simon

Reply via email to