From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bin Meng Date: Mon, 22 Feb 2021 20:13:03 +0800 Subject: [PATCH v2 14/38] test: cmd: Add a basic test for 'addrmap' command In-Reply-To: References: <1613663886-83811-1-git-send-email-bmeng.cn@gmail.com> <1613663886-83811-15-git-send-email-bmeng.cn@gmail.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Simon, On Mon, Feb 22, 2021 at 5:20 PM Simon Glass wrote: > > Hi Bin, > > On Sun, 21 Feb 2021 at 18:55, Bin Meng wrote: > > > > Hi Simon, > > > > On Mon, Feb 22, 2021 at 12:24 AM Simon Glass wrote: > > > > > > Hi Bin, > > > > > > On Sat, 20 Feb 2021 at 06:01, Bin Meng wrote: > > > > > > > > Hi Simon, > > > > > > > > On Sat, Feb 20, 2021 at 7:55 PM Simon Glass wrote: > > > > > > > > > > On Thu, 18 Feb 2021 at 08:59, Bin Meng wrote: > > > > > > > > > > > > This adds a basic test for the newly introduced 'addrmap' command. > > > > > > > > > > > > Signed-off-by: Bin Meng > > > > > > > > > > > > --- > > > > > > > > > > > > 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 > > > > > > > > > > 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