Hi Alper, On Sun, 30 Aug 2020 at 16:38, Alper Nebi Yasak <alpernebiya...@gmail.com> wrote: > > On 30/08/2020 23:37, Simon Glass wrote: > > On Sun, 30 Aug 2020 at 12:17, Alper Nebi Yasak <alpernebiya...@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
