[PATCH] test: pytest: introduce pytest for network, test tftp notfound

[PATCH] test: pytest: introduce pytest for network, test tftp notfound

[Jonas Rebmann]

barebox already has a few pytest integration tests but none of them are
networking related. This type of test requires a network connection to
the barebox device, without the "network" feature the test is marked
xfail. DHCP is assumed. For the case of qemu device we also assume DHCP
to set serverip correctly. In any other case, it is set via console to
that IP of the pytest-host that has the route to the barebox devices IP.
The test asserts on a successful ICMP ping from the test device to the
pytest-host as well as on a "No such file or directory" conversation
from the barebox device to the pytest-host

Signed-off-by: Jonas Rebmann <mail@schlaraffenlan.de>
---
 test/arm/virt@multi_v8_defconfig.yaml |  1 +
 test/openrisc/generic_defconfig.yaml  |  2 +
 test/py/test_network.py               | 69 +++++++++++++++++++++++++++++++++++
 3 files changed, 72 insertions(+)

diff --git a/test/arm/virt@multi_v8_defconfig.yaml b/test/arm/virt@multi_v8_defconfig.yaml
index d8f8ab5cbf..42ce10328d 100644
--- a/test/arm/virt@multi_v8_defconfig.yaml
+++ b/test/arm/virt@multi_v8_defconfig.yaml
@@ -14,6 +14,7 @@ targets:
       BareboxTestStrategy: {}
     features:
       - virtio-mmio
+      - network
     runner:
       tuxmake_arch: arm64
 images:
diff --git a/test/openrisc/generic_defconfig.yaml b/test/openrisc/generic_defconfig.yaml
index 93ba9586c4..56b70b8242 100644
--- a/test/openrisc/generic_defconfig.yaml
+++ b/test/openrisc/generic_defconfig.yaml
@@ -12,6 +12,8 @@ targets:
         prompt: 'barebox@[^:]+:[^ ]+ '
         bootstring: 'commandline:'
       BareboxTestStrategy: {}
+    features:
+      - network
 images:
   barebox: !template "$LG_BUILDDIR/barebox"
 imports:
diff --git a/test/py/test_network.py b/test/py/test_network.py
new file mode 100644
index 0000000000..00bcea3606
--- /dev/null
+++ b/test/py/test_network.py
@@ -0,0 +1,69 @@
+import pytest
+
+from labgrid import driver,Environment
+import socket
+import threading
+
+TFTP_TEST_PORT = 6968
+
+def get_source_addr(destination_ip, destination_port):
+    udp_socket = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
+    udp_socket.connect((destination_ip, destination_port))
+    source_ip, _ = udp_socket.getsockname()
+    return source_ip
+
+def expect_tftp_notfound(filename, listen_port, listen_addr):
+    uname = filename.encode("ascii")
+    messages = [
+        b"\000\001" + uname + b"\000octet\000timeout\0005\000blksize\000512\000tsize\0000\000",
+        b"\000\005\000\001File not found\000",
+    ]
+    udp_socket = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
+    udp_socket.bind((listen_addr, listen_port))
+    udp_socket.settimeout(3)
+    data, addr = udp_socket.recvfrom(1024)
+    udp_socket.sendto(messages[1], addr)
+    udp_socket.close()
+    assert data == messages[0]
+
+def test_skip():
+    pytest.mark.skip("no network adapter available")
+
+def test_barebox_tftp_not_found(barebox, barebox_config, env):
+    # on duts without network feature, this is expected to fail
+    # set xfail_strict=True to enforce specifying the network feature if available
+    if not 'network' in env.get_target_features():
+        pytest.xfail("network feature not specified")
+
+    barebox.run_check("ifup eth0")
+    guestaddr = barebox.run_check("echo $eth0.ipaddr")[0]
+    assert guestaddr != "0.0.0.0"
+
+    listen_addr = "127.0.0.1"
+    if not isinstance(barebox.console, driver.QEMUDriver):
+        # sending an arbitrary udp package to determine the IP towards dut
+        listen_addr = get_source_addr(guestaddr, TFTP_TEST_PORT)
+        barebox.run_check(f"eth0.serverip={listen_addr}")
+
+    barebox.run_check("ping $eth0.serverip", timeout=2)
+
+    tftp_thread = threading.Thread(
+        target=expect_tftp_notfound,
+        name="expect_tftp_notfound",
+        args=("a", TFTP_TEST_PORT, listen_addr),
+    )
+    tftp_thread.daemon = True
+    tftp_thread.start()
+    
+    try:
+        stdout, _, returncode = barebox.run(f"tftp -P {TFTP_TEST_PORT} a", timeout=3)
+        assert returncode == 127
+    except:
+        raise
+    finally:
+        # terminate a timed-out ftpf
+        barebox.console.sendcontrol("c")
+
+    tftp_thread.join()
+    barebox.run_check("ifdown eth0")
+

---
base-commit: 55cf43c442c260846b393c2099d4a72d0ebdbdd9
change-id: 20240605-test_tftp-9e12282d46a0

Best regards,
-- 
Jonas Rebmann <mail@schlaraffenlan.de>

Re: [PATCH] test: pytest: introduce pytest for network, test tftp notfound

[Ahmad Fatoum]

Hello Jonas,

Thanks for your patch!

Some minor comments below.

On 05.06.24 15:13, Jonas Rebmann wrote:
> barebox already has a few pytest integration tests but none of them are
> networking related. This type of test requires a network connection to
> the barebox device, without the "network" feature the test is marked
> xfail. DHCP is assumed. For the case of qemu device we also assume DHCP
> to set serverip correctly. In any other case, it is set via console to
> that IP of the pytest-host that has the route to the barebox devices IP.
> The test asserts on a successful ICMP ping from the test device to the
> pytest-host as well as on a "No such file or directory" conversation
> from the barebox device to the pytest-host

Can you add some line breaks into the commit message for v2?

Also please add following paragraph (or something similar):

  Testing with real hardware and LG_PROXY would fail, but that's arguably
  ok for now.

Just to document the limitation.

> Signed-off-by: Jonas Rebmann <mail@schlaraffenlan.de>
> ---
>  test/arm/virt@multi_v8_defconfig.yaml |  1 +
>  test/openrisc/generic_defconfig.yaml  |  2 +

I wouldn't have expected that so few defconfigs have networking
out of the box. Once we have this first test, it'll be worthwhile
to add support for the other targets too.

>  test/py/test_network.py               | 69 +++++++++++++++++++++++++++++++++++
>  3 files changed, 72 insertions(+)
> 
> diff --git a/test/arm/virt@multi_v8_defconfig.yaml b/test/arm/virt@multi_v8_defconfig.yaml
> index d8f8ab5cbf..42ce10328d 100644
> --- a/test/arm/virt@multi_v8_defconfig.yaml
> +++ b/test/arm/virt@multi_v8_defconfig.yaml
> @@ -14,6 +14,7 @@ targets:
>        BareboxTestStrategy: {}
>      features:
>        - virtio-mmio
> +      - network
>      runner:
>        tuxmake_arch: arm64
>  images:
> diff --git a/test/openrisc/generic_defconfig.yaml b/test/openrisc/generic_defconfig.yaml
> index 93ba9586c4..56b70b8242 100644
> --- a/test/openrisc/generic_defconfig.yaml
> +++ b/test/openrisc/generic_defconfig.yaml
> @@ -12,6 +12,8 @@ targets:
>          prompt: 'barebox@[^:]+:[^ ]+ '
>          bootstring: 'commandline:'
>        BareboxTestStrategy: {}
> +    features:
> +      - network
>  images:
>    barebox: !template "$LG_BUILDDIR/barebox"
>  imports:
> diff --git a/test/py/test_network.py b/test/py/test_network.py
> new file mode 100644
> index 0000000000..00bcea3606
> --- /dev/null
> +++ b/test/py/test_network.py
> @@ -0,0 +1,69 @@
> +import pytest
> +
> +from labgrid import driver,Environment
> +import socket
> +import threading
> +
> +TFTP_TEST_PORT = 6968

Hmm, hardcoding this is a bit unfortunate as it could lead to tests for multiple DUTs
running in parallel to interfere with each other.

> +
> +def get_source_addr(destination_ip, destination_port):
> +    udp_socket = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
> +    udp_socket.connect((destination_ip, destination_port))
> +    source_ip, _ = udp_socket.getsockname()
> +    return source_ip
> +
> +def expect_tftp_notfound(filename, listen_port, listen_addr):
> +    uname = filename.encode("ascii")
> +    messages = [
> +        b"\000\001" + uname + b"\000octet\000timeout\0005\000blksize\000512\000tsize\0000\000",
> +        b"\000\005\000\001File not found\000",
> +    ]
> +    udp_socket = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
> +    udp_socket.bind((listen_addr, listen_port))

If you set listen_port to 0, you get an ephemeral port assigned to avoid
port number clashes.
You'll probably want to move this out of the thread then.

> +    udp_socket.settimeout(3)
> +    data, addr = udp_socket.recvfrom(1024)
> +    udp_socket.sendto(messages[1], addr)
> +    udp_socket.close()
> +    assert data == messages[0]
> +
> +def test_skip():
> +    pytest.mark.skip("no network adapter available")

Unused.

> +def test_barebox_tftp_not_found(barebox, barebox_config, env):
> +    # on duts without network feature, this is expected to fail
> +    # set xfail_strict=True to enforce specifying the network feature if available
> +    if not 'network' in env.get_target_features():
> +        pytest.xfail("network feature not specified")

Nice. Didn't know about xfail.

> +    barebox.run_check("ifup eth0")

I don't think we have any such boards in Qemu, but in general boards
can have multiple Ethernet ports and the first might not even
have a link. Please use ifup -a instead to bring up all interfaces.
If multiple ports have a link, maybe test on all of them, e.g.:

for line in stdout:
    m = re.search('DHCP client bound to address (.*)', line)
    if m is None:
        continue
 
    do_what_you_currently_do(address=m.group(1))

> +    guestaddr = barebox.run_check("echo $eth0.ipaddr")[0]
> +    assert guestaddr != "0.0.0.0"
> +
> +    listen_addr = "127.0.0.1"
> +    if not isinstance(barebox.console, driver.QEMUDriver):
> +        # sending an arbitrary udp package to determine the IP towards dut
> +        listen_addr = get_source_addr(guestaddr, TFTP_TEST_PORT)
> +        barebox.run_check(f"eth0.serverip={listen_addr}")
> +
> +    barebox.run_check("ping $eth0.serverip", timeout=2)
> +
> +    tftp_thread = threading.Thread(
> +        target=expect_tftp_notfound,
> +        name="expect_tftp_notfound",
> +        args=("a", TFTP_TEST_PORT, listen_addr),
> +    )
> +    tftp_thread.daemon = True
> +    tftp_thread.start()
> +    
> +    try:
> +        stdout, _, returncode = barebox.run(f"tftp -P {TFTP_TEST_PORT} a", timeout=3)
> +        assert returncode == 127

returncode != 0. We don't care which error code.

> +    except:
> +        raise

This is unneeded.

> +    finally:
> +        # terminate a timed-out ftpf

s/ftpf/tftp/

> +        barebox.console.sendcontrol("c")
> +
> +    tftp_thread.join()
> +    barebox.run_check("ifdown eth0")

These should be moved into the finally clause too, right?

Cheers,
Ahmad

> +
> 
> ---
> base-commit: 55cf43c442c260846b393c2099d4a72d0ebdbdd9
> change-id: 20240605-test_tftp-9e12282d46a0
> 
> Best regards,

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |