Hi Heinrich, On Wed, 19 Oct 2022 at 15:39, Heinrich Schuchardt <xypron.g...@gmx.de> wrote: > > On 10/19/22 15:17, Simon Glass wrote: > > iHi Heinrich, > > > > On Fri, 14 Oct 2022 at 22:43, Heinrich Schuchardt <xypron.g...@gmx.de> > > wrote: > >> > >> On 10/15/22 03:10, Simon Glass wrote: > >>> Hi Ilias, > >>> > >>> On Fri, 14 Oct 2022 at 09:59, Ilias Apalodimas > >>> <ilias.apalodi...@linaro.org> wrote: > >>>> > >>>> Hi Simon, > >>>> > >>>> On Fri, 14 Oct 2022 at 18:56, Simon Glass <s...@chromium.org> wrote: > >>>>> > >>>>> Hi, > >>>>> > >>>>> On Fri, 14 Oct 2022 at 00:58, Masahisa Kojima > >>>>> <masahisa.koj...@linaro.org> wrote: > >>>>>> > >>>>>> Provide a unit test for the eficonfig secure boot key > >>>>>> management menu. > >>>>>> > >>>>>> Signed-off-by: Masahisa Kojima <masahisa.koj...@linaro.org> > >>>>>> --- > >>>>>> No change since v2 > >>>>>> > >>>>>> newly created in v2 > >>>>>> > >>>>>> test/py/tests/test_eficonfig/conftest.py | 84 +++- > >>>>>> test/py/tests/test_eficonfig/defs.py | 14 + > >>>>>> .../test_eficonfig/test_eficonfig_sbkey.py | 472 > >>>>>> ++++++++++++++++++ > >>>>>> 3 files changed, 568 insertions(+), 2 deletions(-) > >>>>>> create mode 100644 test/py/tests/test_eficonfig/defs.py > >>>>>> create mode 100644 > >>>>>> test/py/tests/test_eficonfig/test_eficonfig_sbkey.py > >>>>> > >>>>> Please can this test be in C? Also, using down-arrow to select menus > >>>>> is brittle. Add a function to select the one you want, e.g. by name. > >>>>> > >>>> > >>>> Is there a very specific reason why we should do stuff like that in C? > >>> > >>> Yes, see here. > >>> > >>>> Python is way easier to extend and test in our case. > >>> > >>> In what way? It seems a lot more complicated, plus the brittle nature > >>> of this test suggests it will be a hassle to maintain. > >>> > >>> https://u-boot.readthedocs.io/en/latest/develop/tests_writing.html#python-or-c > >>> > >>> There is a pending update here too: > >>> > >>> https://patchwork.ozlabs.org/project/uboot/patch/20221013122927.636867-15-...@chromium.org/ > >> > >> The discussion touches different aspects: > >> > >> ** What does it take to make a GUI easily testable? ** > >> > >> Relying on cursor movement for testing is fragile. This is why GUIs > >> often assign to each executable element the following: > >> > >> * Access key, e.g <CTRL><S> > >> A key combination that when entered will have the the same > >> effect as selecting the GUI item > >> * Access string, '@SAVE' > >> A string that when typed into the command field will have > >> the same effect as selecting the GUI item > >> > >> Let's look at U-Boot's menu entry definition: > >> > >> struct bootmenu_entry { > >> unsigned short int num; // unique number 0 .. MAX_COUNT > >> char key[3]; // key identifier of number > >> char *title; // title of entry > >> char *command; // hush command of entry > >> enum boot_type type; // boot type of entry > >> u16 bootorder; // order for each boot type > >> struct bootmenu_data *menu; // this bootmenu > >> struct bootmenu_entry *next; // next menu entry (num+1) > >> } > >> > >> Our structure lacks an accessor element that can be used to select a > >> menu item without using cursor actions. > >> > >> Compound keystrokes like <CTRL><F6> are send as multiple bytes on the > >> console, e.g. 1b 5b 31 37 3b 35 7e. > >> > >> We may define a field shortcut of type char *. If the string is received > >> by the menu loop, let it activate the matching menu entry. Let cursor > >> actions (up, down, enter, space '+', '-') interrupt matching the > >> shortcut string. > >> > >> Instead we could also use a convention for the title: > >> > >> If a letter in the title is preceded by '&', this is the shortcut key. > >> This letter will be shown highlighted in the menu and the ampersand will > >> not be shown. > >> > >> This is probably easier to implement. > >> > >> Adding the shortcut facility will allow both for easier testing and > >> faster navigation. > >> > >> ** Choice of programming language ** > >> > >> Several aspects control the choice of the programming language for tests: > >> > >> - Testing single library functions is only possible in C. > >> - Checking contents of internal structures is only possible in C. > >> - Testing the U-Boot's CLI is easily accessible in our Python framework. > >> - Preparation of complex test data is easier to do in Python. > >> - Mixed language tests should be avoided if not strictly necessary. > >> It is much easier to maintain a single code source for a test. > >> > >> ** What to do for secure boot key management? ** > >> > >> Secure boot key management requires complex preparation which fits well > >> into out Python testing framework. > >> > >> Once we provide shortcut keys to U-Boot menus the entries will be easily > >> accessible from Python. > >> > >> As we want to avoid complexity due to mixed language tests we should > >> stick to Python for testing key management at the user level. > > > > The test setup should be in Python (and needs to be) but the actual > > test should be in C, in general. > > > >>From what I can tell these tests should be split up, one for each test case. > > > > Using keyboard shortcuts for menus would certainly help, but you still > > have the dependency between the menu code in C and the Python code > > that uses it. The complexity of this dependency is quite significant. > > > > Can we not design this all for easier testing? The functionality > > should be broken down a little more so we can change the configs in a > > test (e.g. using the eficonfig command), then check that booting does > > what we expect. > > > > Overall I feel that this 'black box' testing is not a good approach > > and we should try to test the components. It is OK to have a > > happy-path test but here we are just writing hundreds of lines of > > Python to get a very high-level signal about correctness. > > From other projects I am used to differentiate between unit tests and > integration tests. > > An integration test is what checks that all the parts work together > while a unit test checks a single functionality works as designed. > > A description can be found in > https://circleci.com/blog/unit-testing-vs-integration-testing/#c-consent-modal > > For an integration test this Python script is fine (except for the menu > navigation. > > As I said in my last mail additional tests on a lower level may be > added. But unit tests cannot replace an integration test.
We should do the C test first, then. It just has so much more visibility into what is going on. I am OK with an integration test, but it should never find a bug that is not found by the sandbox unit test. The Python tests are much harder to debug! So let's get this sorted out so we don't end up with a complete mess here. Regards, Simon
