On 09/19/2017 02:04 PM, Heinrich Schuchardt wrote:
The necessary parameters for running Python tests on qemu are
tedious to find.

The patch adds a complete example for running the Python tests for
qemu-x86_defconfig on an X86 system.

diff --git a/test/py/README.md b/test/py/README.md

+#### Running tests for qemu-x86\_defconfig on an x86 system
+
+We build u-boot.rom with
+
+    export BUILD_ROM=y
+    make mrproper
+    make qemu-x86_defconfig
+    make

Nit: I'd rather write that as:

BUILD_ROM=y make

That way, we don't have to set BUILD_ROM permanently in the current shell's environment, so it won't affect anything else that's done later in the shell.

To avoid polluting the source tree with build results, why not build with O=./build-qemu-x86, and use that as the --build-dir argument to test/py?

+We create directories `$HOME/ubtest/bin` and `$HOME/ubtest/py` for the script
+files below and `../tftp` for the tftp server.

Why not $HOME/ubtest/tftp? The meaning of "../tftp" can change depending on current directory.

+File `u-boot-test-console` chmod 755
+
+    #!/bin/sh
+    touch /tmp/u-boot-monitor-socket

This creates a regular file not a socket. I believe you can remove this command.

Ah, there's a race condition here. By the time u-boot-test-reset is run the first time, qemu typically hasn't run far enough to create /tmp/u-boot-monitor-socket, and so u-boot-test-reset fails to open it. If I fix that by making u-boot-test-reset wait until the control socket actually does exist, then test/py fails because it sees two signon messages from U-Boot and thinks it crashed; one from when qemu started and booted U-Boot, and one because the first invocation of u-boot-test-reset resets the system which causes another signon to appear. Perhaps this is why Tom Rini's test/py+qemu integration spawns a new instance of qemu every time u-boot-test-console is run, and make u-boot-test-reset a no-op.

To solve this, I was going to say:

a) Add -S to the qemu command. This causes qemu to perform all initialization (e.g. creating/opening the monitor socket) but doesn't actually allow the target to run. This prevents the undesired immediate boot of the system.

b) Add the following to the top of u-boot-test-reset so that it waits for the monitor socket to exist before attempting to use it:

for i in $(seq 50); do
  if [ -f /tmp/u-boot-monitor-socket ]; then
    break
  fi
  sleep 0.1
done

c) Make u-boot-test-reset send U-Boot command "c" to start U-Boot executing instead of or in additionk to "system_reset" to cause a reset.

However, none of that is necessary. u-boot-test-console is re-executed every time test/py wants to connect to the target, which happens (a) when test/py starts the first test, and (b) any time a test fails, and test/py wants to clear all state and reconnect to the target (e.g. the console command might have failed rather than the target, so the console command is restarted too). As such, there's no need to implement u-boot-test-reset at all with qemu; just make it a complete no-op. That way, you also don't need qemu's -monitor at all.

+    exec qemu-system-x86_64 -bios u-boot.rom -nographic -netdev \
+      user,id=eth0,tftp=../tftp,net=192.168.76.0/24,dhcpstart=192.168.76.9 \
+      -device e1000,netdev=eth0 -machine pc-i440fx-2.8 \

Is machine version 2.8 strictly necessary? The qemu packaged in Ubuntu 14.04 doesn't support that version, which restricts the usefulness of the example. Can version 2.0 work for example? It seems to work fine for me. My qemu supports the following i440fx:

qemu-system-x86_64 -machine help | grep i440fx
pc-i440fx-2.0        Standard PC (i440FX + PIIX, 1996)
pc-i440fx-1.4        Standard PC (i440FX + PIIX, 1996)
pc-i440fx-1.5        Standard PC (i440FX + PIIX, 1996)
pc Ubuntu 14.04 PC (i440FX + PIIX, 1996) (alias of pc-i440fx-trusty)
pc-i440fx-trusty     Ubuntu 14.04 PC (i440FX + PIIX, 1996) (default)
pc-i440fx-1.7        Standard PC (i440FX + PIIX, 1996)
pc-i440fx-1.5-saucy Standard PC (i440FX + PIIX, 1996) (alias of pc-i440fx-1.5-qemu-kvm)
pc-i440fx-1.5-qemu-kvm Standard PC (i440FX + PIIX, 1996)
pc-i440fx-1.6        Standard PC (i440FX + PIIX, 1996)

+File `u-boot-test-quit` chmod 755
+
+    #!/bin/sh
+    echo quit | socat - UNIX-CONNECT:/tmp/u-boot-monitor-socket
+
+This script is called when all tests are completed. The `quit` command is
+passed to the qemu control console to terminate the qemu session.

test/py doesn't run u-boot-test-quit. Do you have a local change that makes it do that? My knee-jerk reaction is that it's OK to enhance test/py to run such a script, but let's not document it in the example until it does. That said, I'd assume qemu should exit as soon as test/py exits since stdin will go away, so this shouldn't actually be needed in this case.

+File `u-boot-test-reset` chmod 755
+
+    #!/bin/sh
+    echo system_reset | socat - UNIX-CONNECT:/tmp/u-boot-monitor-socket
+    true

As mentioned previously, the true doesn't appear to be required, and hides legitimate errors.

+In the `$HOME/ubtest/py` directory we create file `u_boot_boardenv_qemu_x86.py`

Since the file mode is mentioned for all the other files, let's mention it here too.

+    env__net_dhcp_server = True
+    env__efi_loader_helloworld_file = {
+      "fn": "helloworld.efi",
+      "size": 4298,
+      "crc32": "55d96ef8",
+    }
+
+The parameter `env__net_dhcp_server` enables the network tests. The parameter
+`env__efi_loader_helloworld_file` is needed to make the file `helloworld.efi`
+available which is loaded from the tftp server in `test_efi_loader.py`.
+
+The fields size and crc32 have to be updated to match the actual values. The
+`crc32` command can be used to determine the latter.

Why not have Python determine those values automatically? Here's a snippet from my uboot-test-hooks repo that does that. Since people will simply be copy-and-pasting the example, it shouldn't matter that it's non-trivial:

{
  "fn": file_name,
  "size": os.path.getsize(file_full),
  "crc32": hex(binascii.crc32(open(file_full, 'rb').read()) & \
    0xffffffff)[2:],
}

+We now can run the Python tests with
+
+    export PATH=$HOME/ubtest/bin:/usr/bin:/bin

Let's not hard-code /usr/bin:/bin; some people may rely on binaries elsewhere. Also I know that the recently added gpt tests rely on sgdisk which is in sbin on my system at least. Also quote to avoid issues with spaces in path names. How about:

PATH="$HOME/ubtest/bin:$PATH"

+    export PYTHONPATH=$HOME/ubtest/py

People might have PYTHONPATH set already (e.g. if using a virtualenv or having installed some packages in their home directory). How about:

PYTHONPATH="$HOME/ubtest/py:$PYTHONPATH"

+    ./test/py/test.py --bd=qemu-x86 --build-dir=.

As above, I'd suggest setting environment variables just for the one command rather than permanently changing them:

PATH="$HOME/ubtest/bin:$PATH" \
  PYTHONPATH="$HOME/ubtest/py:$PYTHONPATH" \
  ./test/py/test.py --bd=qemu-x86 --build-dir=./build-qemu-x86
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to