Am Freitag, dem 19.07.2024 um 11:23 -0400 schrieb Suhail Singh: > Liliana Marie Prikler <liliana.prik...@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. We can test `comp-abi-hash` as well, should be no biggie.
> > + (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 ? Yes, it should. > > + (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)? We could do so, but that'd be an expensive test. In practice, we assume the same layout and just poke a single file. > > +(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.™ You can verify this part by running "make check-system TESTS=emacs- native-comp" and seeing that it fails for this commit but succeeds on the next. That's TDD :D Cheers
