Hi Sughosh, On Tue, 11 Jul 2023 at 01:13, Sughosh Ganu <sughosh.g...@linaro.org> wrote: > > hi Simon, > > On Tue, 11 Jul 2023 at 03:08, Simon Glass <s...@chromium.org> wrote: > > > > Hi Sughosh, > > > > On Sun, 9 Jul 2023 at 07:34, Sughosh Ganu <sughosh.g...@linaro.org> wrote: > > > > > > Add support in binman for generating capsules. The capsule parameters > > > can be specified either through a config file or through the capsule > > > binman entry. > > > > > > Signed-off-by: Sughosh Ganu <sughosh.g...@linaro.org> > > > --- > > > Changes since V2: > > > * New patch which generates capsules through binman replacing the > > > earlier make target. > > > > > > tools/binman/btool/mkeficapsule.py | 91 +++++++++++++++++++++++++ > > > tools/binman/entries.rst | 27 ++++++++ > > > tools/binman/etype/capsule.py | 102 +++++++++++++++++++++++++++++ > > > 3 files changed, 220 insertions(+) > > > create mode 100644 tools/binman/btool/mkeficapsule.py > > > create mode 100644 tools/binman/etype/capsule.py > > > > Please do check test coverage (binman test -T). You are missing quite > > a lot in two two files you have added. > > I was aware of adding tests in binman, but since the capsules > generated through binman are getting tested in the capsule update > functionality, I thought this would be superfluous. If this is > mandatory, I will add the tests. Will also address the rest of your > comments for this patch.
The binman tests are for binman, which is a self-contained program with 100% test coverage. That is a very important feature, since lots of random SoC-specific features depend on it. If something breaks, it can be a bit of a disaster. Also, 100% test coverage allows refactoring of binman's code without worry about breaking something. This is also very important, since we will find problems which the current code base cannot solve. People need the confidence to refactor In practice, the tests define the behaviour. Code which is not tested is therefore considered 'dead' code and should be deleted. A notable point with binman tests is that all failure modes are tested, not just 'happy path'. [..] Regards, Simon
