Subject: Re: [bug#75010] [PATCH v2 1/2] gnu: tests: Add module for guix deploy tests.

From: Ludovic Courtès <[email-masked]>
Date: Sun, 09 Feb 2025 17:31:32 +0100

Herman Rimm <[email-masked]> skribis:

> * gnu/tests/deploy.scm: Add file.
> * gnu/local.mk (GNU_SYSTEM_MODULES): Register file.
>
> Change-Id: I348c8bf2e518ec6c00af126993eaca3fcd453901

Nice!

[...]

> +(define (machines os)

Unless I’m mistaken, ‘os’ is actually unused.  I’d suggest making ‘os’
an sexp and actually using it instead of reading
/run/current-system/configuration.scm.

The OS to deploy should be different from the one already running (at
least a different host name, etc.) as a way to ensure that an actual
deployment occurs.

So you should probably have:

  (define target-os-source
    '(begin
       (use-modules (gnu))
       …
       (operating-system
         …)))

(This is similar to uses of ‘define-os-with-source’ in other tests
except that we probably only need the source in this case.)

> +  (scheme-file
> +    "machines.scm"
> +    #~(begin (use-modules (gnu machine ssh)

Please indent the usual way:

  (scheme-file "machines.scm"
               #~(begin
                   (use-modules …)
                   …))

> +             ;; XXX: (guix platforms ...) are not found in %load-path.
> +             (set! (@ (guix platform) systems)
> +                   (compose list %current-system))

What’s the problem here?

> +                      (machine-ssh-configuration
> +                        (host-name "localhost")
> +                        (host-key
> +                         (string-append
> +                           "ssh-ed25519 "
> +                           (public-key->string
> +                             (public-key-from-file
> +                               "/etc/ssh/ssh_host_ed25519_key.pub"))))

It’s enough to do:

  (call-with-input-file "/etc/ssh/ssh_host_ed25519_key.pub"
    get-string-all)

> +(define %test-deploy
> +  (system-test
> +   (name "deploy")
> +   (description "Deploy to the local machine.")
> +   (value (run-deploy-test))))
> +
> +(define %test-rollback
> +  (system-test
> +   (name "rollback")
> +   (description "Rollback the deployment of a faulty bootloader.")
> +   (value (run-rollback-test))))

For clarity, I’d change ‘name’ of the 2nd one to "deploy-rollback" and
clarify in the description that this is about ‘guix deploy’.

Could you send an updated version?

Thanks,
Ludo’.