Hi Simon,
On Mon, Feb 22, 2021 at 5:20 PM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Sun, 21 Feb 2021 at 18:55, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Mon, Feb 22, 2021 at 12:24 AM Simon Glass sjg@chromium.org wrote:
Hi Bin,
On Sat, 20 Feb 2021 at 06:01, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sat, Feb 20, 2021 at 7:55 PM Simon Glass sjg@chromium.org wrote:
On Thu, 18 Feb 2021 at 08:59, Bin Meng bmeng.cn@gmail.com wrote:
This adds a basic test for the newly introduced 'addrmap' command.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
Changes in v2:
- new patch: test: cmd: Add a basic test for 'addrmap' command
include/test/suites.h | 2 ++ test/cmd/Makefile | 1 + test/cmd/addrmap.c | 38 ++++++++++++++++++++++++++++++++++++++ test/cmd_ut.c | 6 ++++++ 4 files changed, 47 insertions(+) create mode 100644 test/cmd/addrmap.c
Reviewed-by: Simon Glass sjg@chromium.org
Just checking this test is enabled for sandbox?
Not yet. I don't think sandbox has enabled CONFIG_ADDR_MAP.
OK then can you please enable it so we have test coverage?
I am not sure if Sandbox can support CONFIG_ADDR_MAP (non-identity virtual-physical mapping)?
Well it doesn't even really need to support it fully. Just adding the config and writing a test that sets a few entries and checks the functions in addrmap.h do the right thing should be enough. It should be 10 lines of code.
This sounds like another patch to test the library itself instead of the command only. I think we can add the library testing in future patches given Priyanka has reviewed almost all patches in this series.
As Tom mentioned here [1], enabling unit tests on QEMU targets makes more sense?
[1] https://lists.denx.de/pipermail/u-boot/2021-February/441779.html
That was referring to a qemu-specific feature (called into qemu, actually). But in this case, if there is a failure, how will someone diagnose it? Run a huge functional test with qemu to see that it fails somewhere...? I think unit tests are far more useful for little features.
Regards, Bin