Re: [PATCH] release/checks/try.nix: init

Ryan Lahfa
2023-05-27T13:52:42Z

On Fri, May 26, 2023 at 09:07:58PM +0000, Alyssa Ross wrote:
> This is a regression test for c7f87f3 ("host/rootfs: allow growing ext
> partition to fail"). It's the first time we're actually doing
> automated tests of a Spectrum boot. For now, I'm using the NixOS test
> framework, but because we're not using NixOS and not setting any NixOS
> options, it feels to me like it doesn't actually buy us very much, so
> if it doesn't start adding more value as we add more (or more complex)
> tests, it might be simpler to just use a shell/execline script for
> tests.

Are you only interested into tests that works without any
instrumentation?

e.g. what if Spectrum added the backdoor.service in their service
management? That'd repair the ability of the test framework to have
better interactions with the guest without QEMU Agent.

On my side, I am interested supporting testing things like Debian + Nix
via the test framework and saying "here's a disk image or something,
boot it and connect to it".

>
> Signed-off-by: Alyssa Ross <hi@alyssa.is>
> ---
>
> Documentation for reviewing patches:
> https://spectrum-os.org/doc/contributing/reviewing-patches.html
>
> release/checks/default.nix | 2 ++
> release/checks/try.nix | 50 ++++++++++++++++++++++++++++++++++++++
> release/live/default.nix | 2 +-
> 3 files changed, 53 insertions(+), 1 deletion(-)
> create mode 100644 release/checks/try.nix
>
> diff --git a/release/checks/default.nix b/release/checks/default.nix
> index f8a3f9b..bf26b9a 100644
> --- a/release/checks/default.nix
> +++ b/release/checks/default.nix
> @@ -15,4 +15,6 @@ import ../../lib/eval-config.nix ({ ... } @ args:
> rustfmt = import ./rustfmt.nix args;
>
> shellcheck = import ./shellcheck.nix args;
> +
> + try = import ./try.nix args;
> })
> diff --git a/release/checks/try.nix b/release/checks/try.nix
> new file mode 100644
> index 0000000..73539ef
> --- /dev/null
> +++ b/release/checks/try.nix
> @@ -0,0 +1,50 @@
> +# SPDX-License-Identifier: MIT
> +# SPDX-FileCopyrightText: 2023 Alyssa Ross <hi@alyssa.is>
> +
> +import ../../lib/eval-config.nix ({ config, ... }:
> +
> +let
> + live = import ../live { inherit config; };
> +in
> +
> +config.pkgs.nixosTest ({ pkgs, ... }: {
> + name = "try-spectrum-test";
> + nodes = {};
> +
> + testScript = ''
> + import shlex
> + import subprocess
> +
> + conf = subprocess.run([
> + "${pkgs.mtools}/bin/mcopy",
> + "-i",
> + "${live}@@1M",
> + "::loader/entries/spectrum.conf",
> + "-",
> + ], stdout=subprocess.PIPE)
> + conf.check_returncode()
> +
> + cmdline = None
> + for line in conf.stdout.decode('utf-8').splitlines():
> + key, value = line.split(' ', 1)
> + if key == 'options':
> + cmdline = value

Is there any reason to not have `conf` / `cmdline`
being derived in a derivation?

> +
> + flags = " ".join(map(shlex.quote, [
> + "qemu-kvm",
> + "-m", "512",
> + "-kernel", "${live.rootfs.kernel}/${pkgs.stdenv.hostPlatform.linux-kernel.target}",
> + "-initrd", "${live.initramfs}",
> + "-device", "qemu-xhci",
> + "-device", "usb-storage,drive=drive1,removable=true",
> + "-drive", "file=${live},id=drive1,format=raw,if=none,readonly=on",

Does it bring anything that this is real "USB" device? (testing that the
drivers for USB are present?).

In that case, shouldn't testing covers also NVMe devices, etc. ?

> + "-append", f"panic=-1 {cmdline}",
> + ]))
> +
> + machine = create_machine({"startCommand": flags})

`create_machine` is deprecated now, `Machine.create_startcommand` is
"deprecated" too, but I'm not really happy with the fact we do not have
serious replacements to that and I also want to better support "testing
non-NixOS" or non-instrumented systems in the framework.

> +
> + machine.start()
> + machine.wait_for_console_text("EXT4-fs \(sda4\): mounted filesystem")
> + machine.crash()
> + '';
> +}))
> diff --git a/release/live/default.nix b/release/live/default.nix
> index b7ee036..6df73d4 100644
> --- a/release/live/default.nix
> +++ b/release/live/default.nix
> @@ -48,6 +48,6 @@ stdenvNoCC.mkDerivation {
>
> enableParallelBuilding = true;
>
> - passthru = { inherit rootfs; };
> + passthru = { inherit initramfs rootfs; };
> }
> ) {})
> --
> 2.37.1
>

Kind regards,
--
Ryan Lahfa