[PATCH 3/3] binman: Build FIT image subentries with the section etype

[Simon Glass]

Hi Alper,

On Sun, 30 Aug 2020 at 16:38, Alper Nebi Yasak <alpernebiyasak at gmail.com> wrote:
>
> On 30/08/2020 23:37, Simon Glass wrote:
> > On Sun, 30 Aug 2020 at 12:17, Alper Nebi Yasak <alpernebiyasak at gmail.com> wrote:
> >> Entry_section.ObtainContents() never returns False, so I'm removing the
> >> checks for that, which contained the statements the test didn't cover.
> >
> > If you put something in the FIT that depends on something else, it
> > will return False on the first pass. So you can't remove that code.
>
> Section etype already does three passes over its subentries and either
> returns True or raises an exception. Maybe it should return False
> instead, but that breaks the 057_unknown_contents.dts test.

We do want to detect when an entry does not get around to producing
its contents, so this is correct.

>
> > Instead, use the _testing etype with a 'return-contents-later'
> > property. to ensure the path is testing. See 162_fit_external.dts for
> > how this works.
>
> Since section does multiple passes, this appears to only add some more
> data to wherever it's inserted, with no change in coverage.
>
> I can get the exception with 'return-unknown-contents'. If I replace the
> raise with 'return False', it fails in section etype's GetData(). If I
> fix that (section_data += data or b''), the FIT entry returns b'' as its
> entire data due to those checks and only then I can cover them with a
> new test. More trouble than it's worth?

OK I see. But if we later change entry_Section to be more flexible we
will get into trouble. Still, that is what test coverage is for, so I
think it is OK to not check the return value and add a comment as to
why.

Regards,
SImon