Hi,
After long break caused by my vacation, I'm sending version 2 of coreboot unit test proposal, which is a significant rework of unit tests build subsystem. Highlights of most important changes v1->v2:
-> Address all comments from v1;
-> Allow to compile and link more than 2 modules for particular unit tests;
-> Autogeneration of Kconfig for unit tests;
-> Create Makefile.inc tree similar to what is done under src/;
-> Update documentation;
-> Put Makefile and two examples each in a separate commits;
-> Rebase on origin/master.
Thanks for all the comments for v1, looking forward to your remarks.
31 comments:
File Documentation/technotes/2020-03-unit-testing-coreboot.md:
This dependencies
may be broken using mocking concept, to some degree. However when mocks need to
be too complex, then such a unit test cannot address an idea of being quick and
simple.
I like the simple version you propose. […]
Done
and
to some extent hardware-related code
Makes sense.
Done
Patch Set #1, Line 67: Requirements
I agree with Jan; unit tests should build and run on the host. […]
Done
Patch Set #1, Line 79: JUnit-like XML output format for Jenkins
Yes, I think that any machine parsable output should work. […]
Done
Patch Set #1, Line 84: however this is very unlikely
I would re-word this to say that the compiler for the host must support the same language standards […]
Done
Patch Set #1, Line 88: is a plus
Sure. I will add "nice-to-have" items in a separate group.
Done
Patch Set #1, Line 90: The bigger community the better
Explain this a little more. We want a larger community for (fill in the blank). […]
Done
Patch Set #1, Line 92: Language similarity
Similarity to what? It's unclear what you're requiring here.
Done
Patch Set #1, Line 94: Assumption that all firmware devs know C
OK, then state that instead, that unit tests will be written in C because the code being tested is a […]
Done
Patch Set #1, Line 97: Proper
Compatible, or maybe Acceptable
Done
Patrick, […]
I think we can close this thread considering example I have shown. However I haven't contacted with any lawyer, so if you think we should still block this, please let me know.
Patch Set #1, Line 105: for obvious reasons
I agree.
Done
Patch Set #1, Line 142: satisfying
satisfactory
Done
out
Done
Patch Set #1, Line 156: * Easy to integrate with an IDE
Will add this to nice-to-have items.
Done
Patch Set #1, Line 173: * It will require some effort to make it works from within an IDE
Same comment - IDE integration has not been stated as a requirement or a desirable.
Done
Patch Set #1, Line 173: works
work?
Done
Sure. […]
Done
Patch Set #1, Line 179: * There are some (rather easy) hints how to use this from an IDE (e.g. Eclipse)
Same comment - IDE integration has not been stated as a requirement or a desirable.
Done
Cmocka
Done
Patch Set #1, Line 187: proposal is to go with
"we propose using the"
Done
Patch Set #1, Line 188: evaluation criteria we have
stated evaluation criteria
Done
Patch Set #1, Line 190: include
includes
Done
Cmocka also include support
for mocks.
Well, I think once you have unit tests there's always some work to keep them in sync when the tested […]
In v2 I've added "assert.h" mocking header, which replaces dead_code() macro with mock_assert(). This header is actually generic and used through all codebase, so it make sense to put this in for all unit tests.
For the per-tests mock headers, we will need to add an option to append test-cflags before all default coreboot headers.
Patch Set #1, Line 195: In the same time
Sure.
Done
There are some cons, like for example lack of automatic test registration,
however this is minor issue, just requires minimal additional work from
developer.
"Cmocka's limitations, such as the lack of automatic test registration, are considered minor issues […]
Done
That being said, Unity and GoogleTest frameworks are also very good. However
GoogleTest requires C++ familiarity and compiler, and Unity is rather not too
popular framework (which doesn’t mean worse). It is worth to mention, that such
evaluation will always be impacted by author subjective point of view.
Yes, I will remove this. […]
Done
Sure.
Done
Patch Set #1, Line 211: brings
cause
Done
every test file name
should have added a prefix "-test"
Why? Seems like unnecessary boilerplate if the directory structure already makes clear which files a […]
Done
Patch Set #1, Line 232: prefix
> My point with object files' names is that I don't want to mimic directory structure under /build s […]
Seems that we are not on the same page. In general, I was referring to my old proposal, where there were only two object files - test harness and uut. This is no longer valid with v2 completely reworked.
New proposal is to generated per-test dir under build/tests/ and test binary will be placed in the same dir as main test harness (with "-test" suffix). It will be better to discuss inline in v2 code what do you think about my idea.
To view, visit change 39893. To unsubscribe, or for help writing mail filters, visit settings.