bug#72045: [PATCH v2 1/2] gnu: Add system test for Emacs.

[Suhail Singh <suhailsingh247@gmail.com>]

Liliana Marie Prikler <liliana.prikler@gmail.com> writes:

> +          (test-equal "native-comp-dir"
> +            (emacs-native-comp-dir
> +             #$(file-append old-emacs "/bin/emacs"))
> +            (emacs-native-comp-dir
> +             #$(file-append new-emacs "/bin/emacs")))

I like that there is a test that focuses on the native-comp-dir
directly.  Having only a test that focuses on ABI_VERSION wouldn't have
been sufficient IMO.

Minor nitpick: However, there may still be some utility in either having
an additional test for ABI_VERSION or adding a comment that a successful
evaluation of the above test also implies that the ABI_VERSION matches.

> +          (test-assert "old emacs has hierarchical layout"
> +            (file-exists?
> +             (string-append #$new-emacs "/lib/emacs/"
> +                            (emacs-effective-version old-emacs-bin)
> +                            "/native-lisp/"
> +                            (emacs-native-comp-dir old-emacs-bin)
> +                            "/preloaded/emacs-lisp/comp.eln")))

Should that say #$old-emacs instead of #$new-emacs ?

> +          (test-assert "new emacs has hierarchical layout"
> +            (file-exists?
> +             (string-append #$new-emacs "/lib/emacs/"
> +                            (emacs-effective-version new-emacs-bin)
> +                            "/native-lisp/"
> +                            (emacs-native-comp-dir new-emacs-bin)
> +                            "/preloaded/emacs-lisp/comp.eln")))

Do we need to additionally ensure that the new emacs' "hierarchical
layout" matches the old emacs' "hierarchical layout" in some way (over
and above both having them)?

> +(define %test-emacs-native-comp-replacable
> +  (system-test
> +   (name "emacs-native-comp")
> +   (description "Test whether an emacs replacement (if any) is valid.")
> +   (value (run-native-comp-replacable-test
> +           (package-without-replacement emacs)
> +           emacs))))

Ah!  So that's how it's done.  I am not qualified to review this part,
but this looks to be in the right spirit.  Hoping this is merged soon.™

-- 
Suhail