bug#54435: The test mailcap-parsing-and-mailcap-mime-info forgets to restore the HOME environment variable

bug#54435: The test mailcap-parsing-and-mailcap-mime-info forgets to restore the HOME environment variable

[Felix Dietrich]

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

Severity: minor
Tags: patch

Running the test ‘mailcap-parsing-and-mailcap-mime-info’ will set the
HOME environment variable to a temporary directory.  After the test the
directory is removed (thanks to the ‘ert-with-temp-directory’ macro),
but the value of the environment variable not restored, polluting the
state of a running Emacs and affecting later commands in the Emacs
session, like the vc-* commands when git cannot find its global
configuration anymore.

The patch wraps the test in a ‘let’ and ‘unwind-protect’ to restore the
value of the environment variable after the test has been run.


[-- Attachment #2: Patch to ensure restoration of HOME environment variable --]
[-- Type: text/x-diff, Size: 5443 bytes --]

From 137f3fcce83a3f2eadefcc7302fedca86f4ec38a Mon Sep 17 00:00:00 2001
From: Felix Dietrich <felix.dietrich@sperrhaken.name>
Date: Wed, 16 Mar 2022 15:22:51 +0100
Subject: [PATCH] mailcap-tests: Restore HOME environment variable
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

In the test ‘mailcap-parsing-and-mailcap-mime-info’
---
 test/lisp/net/mailcap-tests.el | 83 ++++++++++++++++++----------------
 1 file changed, 43 insertions(+), 40 deletions(-)

diff --git a/test/lisp/net/mailcap-tests.el b/test/lisp/net/mailcap-tests.el
index b439c08c79..2c2955c68f 100644
--- a/test/lisp/net/mailcap-tests.el
+++ b/test/lisp/net/mailcap-tests.el
@@ -78,46 +78,49 @@
    ;; One mailcap entry has a test=false field.  The shell command
    ;; execution errors when running the tests from the Makefile
    ;; because then HOME=/nonexistent.
-   (ert-with-temp-directory home
-     (setenv "HOME" home)
-     ;; Now parse our resource mailcap file.
-     (mailcap-parse-mailcap (ert-resource-file "mailcap"))
-
-     ;; Assert that we get what we have defined.
-     (dolist (type '("audio/ogg" "audio/flac"))
-       (should (string= "mpv %s" (mailcap-mime-info type))))
-     (should (string= "aplay %s" (mailcap-mime-info "audio/x-wav")))
-     (should (string= "emacsclient -t %s"
-                      (mailcap-mime-info "text/plain")))
-     ;; evince is chosen because acroread has test=false and okular
-     ;; comes later.
-     (should (string= "evince %s"
-                      (mailcap-mime-info "application/pdf")))
-     (should (string= "inkscape %s"
-                      (mailcap-mime-info "image/svg+xml")))
-     (should (string= "eog %s"
-                      (mailcap-mime-info "image/jpg")))
-     ;; With REQUEST being a number, all fields of the selected entry
-     ;; should be returned.
-     (should (equal '((viewer . "evince %s")
-                      (type . "application/pdf"))
-                    (mailcap-mime-info "application/pdf" 1)))
-     ;; With 'all, all applicable entries should be returned.
-     (should (equal '(((viewer . "evince %s")
-                       (type . "application/pdf"))
-                      ((viewer . "okular %s")
-                       (type . "application/pdf")))
-                    (mailcap-mime-info "application/pdf" 'all)))
-     (let* ((c nil)
-            (toggle (lambda (_) (setq c (not c)))))
-       (mailcap-add "audio/ogg" "toggle %s" toggle)
-       (should (string= "toggle %s" (mailcap-mime-info "audio/ogg")))
-       ;; The test results are cached, so in order to have the test
-       ;; re-evaluated, one needs to clear the cache.
-       (setq mailcap-viewer-test-cache nil)
-       (should (string= "mpv %s" (mailcap-mime-info "audio/ogg")))
-       (setq mailcap-viewer-test-cache nil)
-       (should (string= "toggle %s" (mailcap-mime-info "audio/ogg")))))))
+   (let ((home (getenv "HOME")))
+     (unwind-protect
+         (ert-with-temp-directory tmp-home
+           (setenv "HOME" tmp-home)
+           ;; Now parse our resource mailcap file.
+           (mailcap-parse-mailcap (ert-resource-file "mailcap"))
+
+           ;; Assert that we get what we have defined.
+           (dolist (type '("audio/ogg" "audio/flac"))
+             (should (string= "mpv %s" (mailcap-mime-info type))))
+           (should (string= "aplay %s" (mailcap-mime-info "audio/x-wav")))
+           (should (string= "emacsclient -t %s"
+                            (mailcap-mime-info "text/plain")))
+           ;; evince is chosen because acroread has test=false and okular
+           ;; comes later.
+           (should (string= "evince %s"
+                            (mailcap-mime-info "application/pdf")))
+           (should (string= "inkscape %s"
+                            (mailcap-mime-info "image/svg+xml")))
+           (should (string= "eog %s"
+                            (mailcap-mime-info "image/jpg")))
+           ;; With REQUEST being a number, all fields of the selected entry
+           ;; should be returned.
+           (should (equal '((viewer . "evince %s")
+                            (type . "application/pdf"))
+                          (mailcap-mime-info "application/pdf" 1)))
+           ;; With 'all, all applicable entries should be returned.
+           (should (equal '(((viewer . "evince %s")
+                             (type . "application/pdf"))
+                            ((viewer . "okular %s")
+                             (type . "application/pdf")))
+                          (mailcap-mime-info "application/pdf" 'all)))
+           (let* ((c nil)
+                  (toggle (lambda (_) (setq c (not c)))))
+             (mailcap-add "audio/ogg" "toggle %s" toggle)
+             (should (string= "toggle %s" (mailcap-mime-info "audio/ogg")))
+             ;; The test results are cached, so in order to have the test
+             ;; re-evaluated, one needs to clear the cache.
+             (setq mailcap-viewer-test-cache nil)
+             (should (string= "mpv %s" (mailcap-mime-info "audio/ogg")))
+             (setq mailcap-viewer-test-cache nil)
+             (should (string= "toggle %s" (mailcap-mime-info "audio/ogg")))))
+       (setenv "HOME" home)))))
 
 (defvar mailcap--test-result nil)
 (defun mailcap--test-viewer ()
-- 
2.35.1


[-- Attachment #3: Type: text/plain, Size: 21 bytes --]



-- 
Felix Dietrich

bug#54435: The test mailcap-parsing-and-mailcap-mime-info forgets to restore the HOME environment variable

[Lars Ingebrigtsen]

Felix Dietrich <felix.dietrich@sperrhaken.name> writes:

> The patch wraps the test in a ‘let’ and ‘unwind-protect’ to restore the
> value of the environment variable after the test has been run.

Thanks.  I've now applied a similar change to Emacs 29, but used
`with-environment-variables' instead of let/unwind-protect.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no