Hi Carl-Daniel,
Thank you so much for reading the projects list and for sharing your thoughts!
It seems you touched two topics in your email: first one is feedback on gsoc projects, and second one is unit tests vs testing on real hardware.
In general, the project descriptions at this stage do not need to be fully detailed. We will certainly get there a bit later, when discussing projects with potential contributors. And your feedback is really important.
Missing context:
… Emulate hardware and filesystem in platform-independent way for unit tests.
“For unit tests” meant to be the context :) Dummy is used in unit tests wherever it makes sense, however sometimes it is not enough. For example, as you said, it is important to test flashrom hardware drivers, but this can’t be done with dummy. This project specifically, is continuing existing effort, so yes there will be good guidance for students and also examples of what’s already done.
I do acknowledge that flashrom
has gotten lots of refactoring, but I'm a bit afraid the changelogs suggest that in many cases "TEST=builds" seems to have been the only test that was performed. It's a good thing to have unit tests, but tests on real hardware should be performed if a driver has been refactored. After all, people rely on flashrom to not break their hardware.
Unit tests are not the replacement of tests on real hardware. However, there is a class of code errors that unit tests can catch (and human eye can miss), and so code is in better shape when it comes to hardware testing. Also, just to be fair, for *some* of the patches unit tests are sufficient. For some, not for all of them, of course.
On Sat, Feb 19, 2022 at 1:14 AM Nico Huber nico.h@gmx.de wrote:
Hi Carl-Daniel,
On 04.02.22 21:59, Carl-Daniel Hailfinger wrote:
I'm glad that there is an effort to participate in GSoC again and I hope it will be successful.
Reading through the current GSoC ideas list, I would like to share a few thoughts.
Unfortunately, some of these ideas are missing a bit of context or may cause needless code churn.
please don't see the project ideas as a detailed manual what a student should do. Sure, we could provide more details, but I guess there should be a balance. IMO, there should be enough room for a student to bring forward their own ideas. Eventually, all details need to be discussed before the implementation is started, of course.
Missing context:
- Design and implement new CLI based on libflashrom and extend the API
as necessary. That would be the fourth or fifth user interface for flashrom. Now if you mean cli_classic.c should be rewritten to use libflashrom, that's something different. The idea doesn't specify more details, but please note that any CLI will have to be supported for the next 20 years and subsequent generations of developers may not be willing to do that.
I don't see a problem with additional CLIs. It depends on their purpose. The idea is not to implement another flashrom frontend that supports all features of cli_classic, but rather to allow simpler implementations that are dedicated to specific use cases. For instance, a CLI that copies data from / to a single flash region. With a stable API and command-line syntax that would need close to 0 maintenance compared to an all-in-one CLI.
- Emulate hardware and filesystem in platform-independent way for unit
tests. flashrom has builtin code for hardware emulation. Some more hardware is emulated quite well by qemu. There were various patches for qemu floating around to help testing flashrom hardware drivers, but someone (TM) would have to dig through our mailing list as well as others.
Well, you generally don't want to start an emulator within a unit test, do you? :) IIRC, the idea is simply to implement whatever is necessary to test a single function, for instance the `command()` function of a SPI master. I think I once called that emulation if you want to do it without the programmer hardware, but you could as well call it mocking.
Some more guidance for students is definitely necessary.
Yes, of course. IMO, that can happen interactively.
Code churn:
- Refactor arguments parser. The rationale given is "Strings are
complicated in C to work with. However, the idea of this is to deduplicate a lot of code.". I agree that strings in C need careful programming. That's also a reason why I doubt rewriting the code would be a good idea. This idea in particular seems to suggest that reinventing a full-blown argument parser building bespoke structures is a way to reduce complexity. In the past, we had proposals to store configuration data and even command line arguments in JSON or XML, then use a system library to parse it. The reasoning for the XML/JSON was that building your own parser was a bad idea. Someone else will come along and refactor things again if we merge a bespoke parser now. "Hey, thanks for your contribution, it will be replaced in the next GSoC" is not something we would want to advertise.
I don't think is going to explode. Nobody said something about drawing non-standard dependencies in. We just want to avoid further code duplication. Like most programmer code that takes arguments follows the same pattern to process it, e.g. check if it's an integer, if it's in range etc. One could also call it the natural evolution of extract_programmer_param(). So basically, do what we always did, but with a little more strategy.
I do acknowledge that flashrom has gotten lots of refactoring, but I'm a bit afraid the changelogs suggest that in many cases "TEST=builds" seems to have been the only test that was performed. It's a good thing to have unit tests, but tests on real hardware should be performed if a driver has been refactored. After all, people rely on flashrom to not break their hardware.
I'm also concerned about the current, real-hardware test coverage. However, personally, I draw a line between changes that affect internal "BIOS" programming and those that don't. Internal programming must work flawlessly. OTOH, external programmers are less scary, worst case (e.g. downgrading flashrom is not an option) somebody needs to apply a hot- fix. If there are big concerns about the internal programmers, we can always limit refactoring to external ones. Actually, to avoid future code duplication, we wouldn't have to touch existing drivers at all. But it also has its benefits to have a uniform code base.
Nico