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

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

[Alyssa Ross]

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.

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
+
+    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",
+      "-append", f"panic=-1 {cmdline}",
+    ]))
+
+    machine = create_machine({"startCommand": flags})
+
+    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

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

[Ryan Lahfa]

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

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

[Alyssa Ross]

[-- Attachment #1: Type: text/plain, Size: 4363 bytes --]

Ryan Lahfa <ryan@lahfa.xyz> writes:

> 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.

At least until I really need tests that depend on guest cooperation, yes.
Because to have either the backdoor service or the QEMU agent, I'd
either have to build custom images just for testing (which would mean
the real images were not actually tested), or I'd have to build those
things into all Spectrum images.

At some point, it might not be possible to get away from this, but the
basic tests I've written so far have all gone fine without the need for
any guest cooperation beyond a serial shell.

> 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".

You might be interested in replying to this Discourse topic:
https://discourse.nixos.org/t/running-a-nixos-test-on-a-different-image/28458

>> +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?

We can't know it at eval time, because it includes the verity hash.  So
our options are to recompute it here, or reuse the results of the
derivation that already computed it, which is the approach I've taken
here.  It's a bit unweildy, but I blame Python for that.  If we do end
up switching to shell it'd just be:

	mcopy -i ${live}@@1M ::loader/entries/spectrum.conf - | sed -n 's/^options //p'

>> +
>> +    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. ?

USB is special in that USB devices don't show up until after userspace
has started, so userspace has to handle waiting for them.  This is not
generally the case with other block devices.  When working on the
installer, I always test with USB, both to ensure this extra complexity
is handled correctly, and because it's what I expect to be the most
common device type for the combined installer image to be booted from in
real life.

>> +      "-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.

There's no viable alternative, so I think the best course of action here
is to undeprecated it:
https://github.com/NixOS/nixpkgs/pull/234427

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

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

[Ryan Lahfa]

On Sat, May 27, 2023 at 02:38:22PM +0000, Alyssa Ross wrote:
> Ryan Lahfa <ryan@lahfa.xyz> writes:
> 
> > 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.
> 
> At least until I really need tests that depend on guest cooperation, yes.
> Because to have either the backdoor service or the QEMU agent, I'd
> either have to build custom images just for testing (which would mean
> the real images were not actually tested), or I'd have to build those
> things into all Spectrum images.
> 
> At some point, it might not be possible to get away from this, but the
> basic tests I've written so far have all gone fine without the need for
> any guest cooperation beyond a serial shell.

Makes sense, supporting "minimal" friction for this usecase is
desirable.

I guess we could have Python *and* Bash/execline as "test scripts" if we want.

But I would need to see more about what would Bash/execline-oriented
test scripts would look like in practice.

Python is suboptimal for various things but the "batteries included" are
very useful for getting a OK-grade thing running.

> > 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".
> 
> You might be interested in replying to this Discourse topic:
> https://discourse.nixos.org/t/running-a-nixos-test-on-a-different-image/28458
> 
> >> +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?
> 
> We can't know it at eval time, because it includes the verity hash.  So
> our options are to recompute it here, or reuse the results of the
> derivation that already computed it, which is the approach I've taken
> here.  It's a bit unweildy, but I blame Python for that.  If we do end
> up switching to shell it'd just be:
> 
> 	mcopy -i ${live}@@1M ::loader/entries/spectrum.conf - | sed -n 's/^options //p'

Makes sense, but do you need it at evaltime?

Wouldn't this be a possibility:

```nix
let optionsFile = pkgs.runCommand "extract-options" {} ''mcopy -i ${live}@@1M ::loader/entries/spectrum.conf - | sed -n 's/^options //p' > $out'';
in
''
  with open(${optionsFile}, "r") as f:
    options = f.read()
''
```

Ideally, with the https://github.com/NixOS/nixpkgs/issues/156563
proposal, this could even become easier IMHO.

> 
> >> +
> >> +    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. ?
> 
> USB is special in that USB devices don't show up until after userspace
> has started, so userspace has to handle waiting for them.  This is not
> generally the case with other block devices.  When working on the
> installer, I always test with USB, both to ensure this extra complexity
> is handled correctly, and because it's what I expect to be the most
> common device type for the combined installer image to be booted from in
> real life.

Got it. :)

> 
> >> +      "-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.
> 
> There's no viable alternative, so I think the best course of action here
> is to undeprecated it:
> https://github.com/NixOS/nixpkgs/pull/234427

Agreed.


Kind regards,
-- 
Ryan Lahfa

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

[Alyssa Ross]

[-- Attachment #1: Type: text/plain, Size: 4561 bytes --]

Ryan Lahfa <ryan@lahfa.xyz> writes:

> On Sat, May 27, 2023 at 02:38:22PM +0000, Alyssa Ross wrote:
>> Ryan Lahfa <ryan@lahfa.xyz> writes:
>> 
>> > 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.
>> 
>> At least until I really need tests that depend on guest cooperation, yes.
>> Because to have either the backdoor service or the QEMU agent, I'd
>> either have to build custom images just for testing (which would mean
>> the real images were not actually tested), or I'd have to build those
>> things into all Spectrum images.
>> 
>> At some point, it might not be possible to get away from this, but the
>> basic tests I've written so far have all gone fine without the need for
>> any guest cooperation beyond a serial shell.
>
> Makes sense, supporting "minimal" friction for this usecase is
> desirable.
>
> I guess we could have Python *and* Bash/execline as "test scripts" if we want.
>
> But I would need to see more about what would Bash/execline-oriented
> test scripts would look like in practice.
>
> Python is suboptimal for various things but the "batteries included" are
> very useful for getting a OK-grade thing running.

I don't think there's much of a reason to support shell/execline scripts
for NixOS tests.  My point here is more that I'm not really getting
anything out of the NixOS test setup at all.  I already have to
construct my own QEMU command line, so the only thing I'm really getting
out of it is the wait_for_console_text() function, which wouldn't be
_that_ hard to do without in a shell/execline script either.

It would probably be different if I _was_ using a guest agent / backdoor
service, so I do think there's value to the NixOS test framework trying
to support other distros.  It would be useful when the tests would
benefit from its built-in systemd integration, or OCR functionality, etc.

>> >> +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?
>> 
>> We can't know it at eval time, because it includes the verity hash.  So
>> our options are to recompute it here, or reuse the results of the
>> derivation that already computed it, which is the approach I've taken
>> here.  It's a bit unweildy, but I blame Python for that.  If we do end
>> up switching to shell it'd just be:
>> 
>> 	mcopy -i ${live}@@1M ::loader/entries/spectrum.conf - | sed -n 's/^options //p'
>
> Makes sense, but do you need it at evaltime?
>
> Wouldn't this be a possibility:
>
> ```nix
> let optionsFile = pkgs.runCommand "extract-options" {} ''mcopy -i ${live}@@1M ::loader/entries/spectrum.conf - | sed -n 's/^options //p' > $out'';
> in
> ''
>   with open(${optionsFile}, "r") as f:
>     options = f.read()
> ''
> ```
>
> Ideally, with the https://github.com/NixOS/nixpkgs/issues/156563
> proposal, this could even become easier IMHO.

Well yeah, I could, but I think that's just extra complexity over doing
it in the driver script.  It's not _that_ bad in Python.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

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

[Ryan Lahfa]

On Sat, May 27, 2023 at 03:07:20PM +0000, Alyssa Ross wrote:
> Ryan Lahfa <ryan@lahfa.xyz> writes:
> 
> > On Sat, May 27, 2023 at 02:38:22PM +0000, Alyssa Ross wrote:
> >> Ryan Lahfa <ryan@lahfa.xyz> writes:
> >> 
> >> > 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.
> >> 
> >> At least until I really need tests that depend on guest cooperation, yes.
> >> Because to have either the backdoor service or the QEMU agent, I'd
> >> either have to build custom images just for testing (which would mean
> >> the real images were not actually tested), or I'd have to build those
> >> things into all Spectrum images.
> >> 
> >> At some point, it might not be possible to get away from this, but the
> >> basic tests I've written so far have all gone fine without the need for
> >> any guest cooperation beyond a serial shell.
> >
> > Makes sense, supporting "minimal" friction for this usecase is
> > desirable.
> >
> > I guess we could have Python *and* Bash/execline as "test scripts" if we want.
> >
> > But I would need to see more about what would Bash/execline-oriented
> > test scripts would look like in practice.
> >
> > Python is suboptimal for various things but the "batteries included" are
> > very useful for getting a OK-grade thing running.
> 
> I don't think there's much of a reason to support shell/execline scripts
> for NixOS tests.  My point here is more that I'm not really getting
> anything out of the NixOS test setup at all.  I already have to
> construct my own QEMU command line, so the only thing I'm really getting
> out of it is the wait_for_console_text() function, which wouldn't be
> _that_ hard to do without in a shell/execline script either.
> 
> It would probably be different if I _was_ using a guest agent / backdoor
> service, so I do think there's value to the NixOS test framework trying
> to support other distros.  It would be useful when the tests would
> benefit from its built-in systemd integration, or OCR functionality, etc.

Got it.

> >> >> +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?
> >> 
> >> We can't know it at eval time, because it includes the verity hash.  So
> >> our options are to recompute it here, or reuse the results of the
> >> derivation that already computed it, which is the approach I've taken
> >> here.  It's a bit unweildy, but I blame Python for that.  If we do end
> >> up switching to shell it'd just be:
> >> 
> >> 	mcopy -i ${live}@@1M ::loader/entries/spectrum.conf - | sed -n 's/^options //p'
> >
> > Makes sense, but do you need it at evaltime?
> >
> > Wouldn't this be a possibility:
> >
> > ```nix
> > let optionsFile = pkgs.runCommand "extract-options" {} ''mcopy -i ${live}@@1M ::loader/entries/spectrum.conf - | sed -n 's/^options //p' > $out'';
> > in
> > ''
> >   with open(${optionsFile}, "r") as f:
> >     options = f.read()
> > ''
> > ```
> >
> > Ideally, with the https://github.com/NixOS/nixpkgs/issues/156563
> > proposal, this could even become easier IMHO.
> 
> Well yeah, I could, but I think that's just extra complexity over doing
> it in the driver script.  It's not _that_ bad in Python.

Got it :).


Kind regards,
-- 
Ryan Lahfa

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

[Ryan Lahfa]

Reviewed-by: Ryan Lahfa <ryan@lahfa.xyz>

On Sat, May 27, 2023 at 03:07:20PM +0000, Alyssa Ross wrote:
> Ryan Lahfa <ryan@lahfa.xyz> writes:
> 
> > On Sat, May 27, 2023 at 02:38:22PM +0000, Alyssa Ross wrote:
> >> Ryan Lahfa <ryan@lahfa.xyz> writes:
> >> 
> >> > 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.
> >> 
> >> At least until I really need tests that depend on guest cooperation, yes.
> >> Because to have either the backdoor service or the QEMU agent, I'd
> >> either have to build custom images just for testing (which would mean
> >> the real images were not actually tested), or I'd have to build those
> >> things into all Spectrum images.
> >> 
> >> At some point, it might not be possible to get away from this, but the
> >> basic tests I've written so far have all gone fine without the need for
> >> any guest cooperation beyond a serial shell.
> >
> > Makes sense, supporting "minimal" friction for this usecase is
> > desirable.
> >
> > I guess we could have Python *and* Bash/execline as "test scripts" if we want.
> >
> > But I would need to see more about what would Bash/execline-oriented
> > test scripts would look like in practice.
> >
> > Python is suboptimal for various things but the "batteries included" are
> > very useful for getting a OK-grade thing running.
> 
> I don't think there's much of a reason to support shell/execline scripts
> for NixOS tests.  My point here is more that I'm not really getting
> anything out of the NixOS test setup at all.  I already have to
> construct my own QEMU command line, so the only thing I'm really getting
> out of it is the wait_for_console_text() function, which wouldn't be
> _that_ hard to do without in a shell/execline script either.
> 
> It would probably be different if I _was_ using a guest agent / backdoor
> service, so I do think there's value to the NixOS test framework trying
> to support other distros.  It would be useful when the tests would
> benefit from its built-in systemd integration, or OCR functionality, etc.
> 
> >> >> +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?
> >> 
> >> We can't know it at eval time, because it includes the verity hash.  So
> >> our options are to recompute it here, or reuse the results of the
> >> derivation that already computed it, which is the approach I've taken
> >> here.  It's a bit unweildy, but I blame Python for that.  If we do end
> >> up switching to shell it'd just be:
> >> 
> >> 	mcopy -i ${live}@@1M ::loader/entries/spectrum.conf - | sed -n 's/^options //p'
> >
> > Makes sense, but do you need it at evaltime?
> >
> > Wouldn't this be a possibility:
> >
> > ```nix
> > let optionsFile = pkgs.runCommand "extract-options" {} ''mcopy -i ${live}@@1M ::loader/entries/spectrum.conf - | sed -n 's/^options //p' > $out'';
> > in
> > ''
> >   with open(${optionsFile}, "r") as f:
> >     options = f.read()
> > ''
> > ```
> >
> > Ideally, with the https://github.com/NixOS/nixpkgs/issues/156563
> > proposal, this could even become easier IMHO.
> 
> Well yeah, I could, but I think that's just extra complexity over doing
> it in the driver script.  It's not _that_ bad in Python.



-- 
Ryan Lahfa

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

[Alyssa Ross]

This patch has been committed as 6b34ec8a8ba7c48837edcec06be808d773ce793a,
which can be viewed online at
https://spectrum-os.org/git/spectrum/commit/?id=6b34ec8a8ba7c48837edcec06be808d773ce793a.

This is an automated message.  Send comments/questions/requests to:
Alyssa Ross <hi@alyssa.is>