Hi Stefano, > On Dec 8, 2023, at 09:41, Stefano Stabellini <sstabell...@kernel.org> wrote: > > On Thu, 7 Dec 2023, Henry Wang wrote: >> This commit adds the shell script for the FVP smoke test. Similarly >> as the QEMU jobs, the shell script will firstly prepare the DomU >> BusyBox image, use the ImageBuilder to arrange the binaries in memory >> and generate the U-Boot script, then start the test. To provide the >> TFTP service for the FVP, the shell script will also start the TFTP >> service, and copy the binaries needed by test to the TFTP directory >> used by the TFTP server. >> >> Signed-off-by: Henry Wang <henry.w...@arm.com> >> --- >> .../scripts/fvp-base-smoke-dom0-arm64.sh | 117 ++++++++++++++++++ >> 1 file changed, 117 insertions(+) >> create mode 100755 automation/scripts/fvp-base-smoke-dom0-arm64.sh > > This script is clear and it is fine: > > Reviewed-by: Stefano Stabellini <sstabell...@kernel.org>
Thanks! > > My only concern is that the expect checks on what booted (Xen, Dom0, > DomU) are inside the script fvp-base-smoke-dom0-arm64.exp rather than in > this script. So if in the future we are going to have multiple tests > with different configurations (for instance see > qemu-smoke-dom0less-arm64.sh) we'll have to find a way to reuse > fvp-base-smoke-dom0-arm64.exp somehow. We do have ways to reuse expect script, for example, we can extract the common code (variables and test logic) to the common.tcl file and source the common.tcl file in the expect script to reuse the code. The reason why I didn’t do that in this series is that currently there is only one script so I feel that there is not much benefit to do this instantly :) Let’s wait to see if there is more comments from others, and I am definitely open to refactor if there is the need to extract the common logic (for example when we add dom0less tests in the future). Kind regards, Henry
