GNU bug report logs - #65143
dired-aux-tests and kmacro-tests failures on master

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 65143 in the body.
You can then email your comments to 65143 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattias.engdegard <at> gmail.com>
To: Emacs Bug Report <bug-gnu-emacs <at> gnu.org>
Cc: Po Lu <luangruo <at> yahoo.com>
Subject: dired-aux-tests and kmacro-tests failures on master
Date: Tue, 8 Aug 2023 11:26:38 +0200

Currently (eeda9eff1a) on master:

   FAILED  dired-test-bug30624
   FAILED  kmacro-tests-step-edit-append
   FAILED  kmacro-tests-step-edit-replace

from dired-aux-tests and macro-tests respectively.

Most likely suspects are the recent Android merge and the unprompted delete-file changes, but I really have no idea.

Message #8 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Po Lu <luangruo <at> yahoo.com>
To: Mattias Engdegård <mattias.engdegard <at> gmail.com>
Cc: Emacs Bug Report <bug-gnu-emacs <at> gnu.org>
Subject: Re: dired-aux-tests and kmacro-tests failures on master
Date: Tue, 08 Aug 2023 17:42:53 +0800

Mattias Engdegård <mattias.engdegard <at> gmail.com> writes:

> Currently (eeda9eff1a) on master:
>
>    FAILED  dired-test-bug30624
>    FAILED  kmacro-tests-step-edit-append
>    FAILED  kmacro-tests-step-edit-replace
>
> from dired-aux-tests and macro-tests respectively.
>
> Most likely suspects are the recent Android merge and the unprompted delete-file changes, but I really have no idea.

Does this change fix the Dired test?

I suspect the test itself is wrong; the doc string of
dired-do-create-files mentions nothing about its return value, which the
test asserts against:

            (dired-mark-files-regexp "bug30624_file")
            (should (dired-do-create-files 'copy 'dired-copy-file "Copy" nil))) <===============
        (delete-directory target-dir 'recursive)

diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
index 3e8b4c3c8fc..825e25c09ed 100644
--- a/lisp/dired-aux.el
+++ b/lisp/dired-aux.el
@@ -2480,87 +2480,88 @@ dired-do-create-files
 
    For any other return value, TARGET is treated as a directory."
   (or op1 (setq op1 operation))
-  (let* ((fn-list (dired-get-marked-files nil arg nil nil t))
-	 (rfn-list (mapcar #'dired-make-relative fn-list))
-	 (dired-one-file	; fluid variable inside dired-create-files
-	  (and (consp fn-list) (null (cdr fn-list)) (car fn-list)))
-	 (target-dir (dired-dwim-target-directory))
-	 (default (and dired-one-file
-		       (not dired-dwim-target) ; Bug#25609
-		       (expand-file-name (file-name-nondirectory (car fn-list))
-					 target-dir)))
-	 (defaults (dired-dwim-target-defaults fn-list target-dir))
-	 (target (expand-file-name ; fluid variable inside dired-create-files
-		  (minibuffer-with-setup-hook
-		      (lambda ()
-                        (setq-local minibuffer-default-add-function nil)
-			(setq minibuffer-default defaults))
-		    (dired-mark-read-file-name
-                     (format "%s %%s %s: "
-                             (if dired-one-file op1 operation)
-                             (if (memq op-symbol '(symlink hardlink))
-                                 ;; Linking operations create links
-                                 ;; from the prompted file name; the
-                                 ;; other operations copy (etc) to the
-                                 ;; prompted file name.
-                                 "from" "to"))
-		     target-dir op-symbol arg rfn-list default))))
-	 (into-dir
-          (progn
-            (when
-                (or
-                 (not dired-one-file)
-                 (and dired-create-destination-dirs-on-trailing-dirsep
-                      (directory-name-p target)))
-              (dired-maybe-create-dirs target))
-            (cond ((null how-to)
-		   ;; Allow users to change the letter case of
-		   ;; a directory on a case-insensitive
-		   ;; filesystem.  If we don't test these
-		   ;; conditions up front, file-directory-p
-		   ;; below will return t on a case-insensitive
-		   ;; filesystem, and Emacs will try to move
-		   ;; foo -> foo/foo, which fails.
-		   (if (and (file-name-case-insensitive-p (car fn-list))
-			    (eq op-symbol 'move)
-			    dired-one-file
-			    (string= (downcase
-				      (expand-file-name (car fn-list)))
-				     (downcase
-				      (expand-file-name target)))
-			    (not (string=
-				  (file-name-nondirectory (car fn-list))
-				  (file-name-nondirectory target))))
-		       nil
-		     (file-directory-p target)))
-		  ((eq how-to t) nil)
-		  (t (funcall how-to target))))))
-    (if (and (consp into-dir) (functionp (car into-dir)))
-	(apply (car into-dir) operation rfn-list fn-list target (cdr into-dir))
-      (if (not (or dired-one-file into-dir))
-	  (error "Marked %s: target must be a directory: %s" operation target))
-      (if (and (not (file-directory-p (car fn-list)))
-               (not (file-directory-p target))
-               (directory-name-p target))
-          (error "%s: Target directory does not exist: %s" operation target))
-      ;; rename-file bombs when moving directories unless we do this:
-      (or into-dir (setq target (directory-file-name target)))
-      (prog1
-          (dired-create-files
-           file-creator operation fn-list
-           (if into-dir			; target is a directory
-	       ;; This function uses fluid variable target when called
-	       ;; inside dired-create-files:
-	       (lambda (from)
-	         (expand-file-name (file-name-nondirectory from) target))
-	     (lambda (_from) target))
-           marker-char)
-        (when (or (eq dired-do-revert-buffer t)
-                  (and (functionp dired-do-revert-buffer)
-                       (funcall dired-do-revert-buffer target)))
-          (dired-fun-in-all-buffers (file-name-directory target) nil
-                                    #'revert-buffer)))))
-  (dired-post-do-command))
+  (prog1
+      (let* ((fn-list (dired-get-marked-files nil arg nil nil t))
+	     (rfn-list (mapcar #'dired-make-relative fn-list))
+	     (dired-one-file	; fluid variable inside dired-create-files
+	      (and (consp fn-list) (null (cdr fn-list)) (car fn-list)))
+	     (target-dir (dired-dwim-target-directory))
+	     (default (and dired-one-file
+		           (not dired-dwim-target) ; Bug#25609
+		           (expand-file-name (file-name-nondirectory (car fn-list))
+					     target-dir)))
+	     (defaults (dired-dwim-target-defaults fn-list target-dir))
+	     (target (expand-file-name ; fluid variable inside dired-create-files
+		      (minibuffer-with-setup-hook
+		          (lambda ()
+                            (setq-local minibuffer-default-add-function nil)
+			    (setq minibuffer-default defaults))
+		        (dired-mark-read-file-name
+                         (format "%s %%s %s: "
+                                 (if dired-one-file op1 operation)
+                                 (if (memq op-symbol '(symlink hardlink))
+                                     ;; Linking operations create links
+                                     ;; from the prompted file name; the
+                                     ;; other operations copy (etc) to the
+                                     ;; prompted file name.
+                                     "from" "to"))
+		         target-dir op-symbol arg rfn-list default))))
+	     (into-dir
+              (progn
+                (when
+                    (or
+                     (not dired-one-file)
+                     (and dired-create-destination-dirs-on-trailing-dirsep
+                          (directory-name-p target)))
+                  (dired-maybe-create-dirs target))
+                (cond ((null how-to)
+		       ;; Allow users to change the letter case of
+		       ;; a directory on a case-insensitive
+		       ;; filesystem.  If we don't test these
+		       ;; conditions up front, file-directory-p
+		       ;; below will return t on a case-insensitive
+		       ;; filesystem, and Emacs will try to move
+		       ;; foo -> foo/foo, which fails.
+		       (if (and (file-name-case-insensitive-p (car fn-list))
+			        (eq op-symbol 'move)
+			        dired-one-file
+			        (string= (downcase
+				          (expand-file-name (car fn-list)))
+				         (downcase
+				          (expand-file-name target)))
+			        (not (string=
+				      (file-name-nondirectory (car fn-list))
+				      (file-name-nondirectory target))))
+		           nil
+		         (file-directory-p target)))
+		      ((eq how-to t) nil)
+		      (t (funcall how-to target))))))
+        (if (and (consp into-dir) (functionp (car into-dir)))
+	    (apply (car into-dir) operation rfn-list fn-list target (cdr into-dir))
+          (if (not (or dired-one-file into-dir))
+	      (error "Marked %s: target must be a directory: %s" operation target))
+          (if (and (not (file-directory-p (car fn-list)))
+                   (not (file-directory-p target))
+                   (directory-name-p target))
+              (error "%s: Target directory does not exist: %s" operation target))
+          ;; rename-file bombs when moving directories unless we do this:
+          (or into-dir (setq target (directory-file-name target)))
+          (prog1
+              (dired-create-files
+               file-creator operation fn-list
+               (if into-dir			; target is a directory
+	           ;; This function uses fluid variable target when called
+	           ;; inside dired-create-files:
+	           (lambda (from)
+	             (expand-file-name (file-name-nondirectory from) target))
+	         (lambda (_from) target))
+               marker-char)
+            (when (or (eq dired-do-revert-buffer t)
+                      (and (functionp dired-do-revert-buffer)
+                           (funcall dired-do-revert-buffer target)))
+              (dired-fun-in-all-buffers (file-name-directory target) nil
+                                        #'revert-buffer)))))
+    (dired-post-do-command)))
 
 ;; Read arguments for a marked-files command that wants a file name,
 ;; perhaps popping up the list of marked files.

Message #11 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattias.engdegard <at> gmail.com>
To: Po Lu <luangruo <at> yahoo.com>
Cc: Emacs Bug Report <bug-gnu-emacs <at> gnu.org>
Subject: Re: dired-aux-tests and kmacro-tests failures on master
Date: Tue, 8 Aug 2023 12:32:51 +0200

8 aug. 2023 kl. 11.42 skrev Po Lu <luangruo <at> yahoo.com>:

> Does this change fix the Dired test?

That should do it, thank you. I pushed a slightly worse-but-better variant to avoid the large `prog1`, but it's essentially the same.

> I suspect the test itself is wrong; the doc string of
> dired-do-create-files mentions nothing about its return value, which the
> test asserts against

I agree, the return value should be documented or the test use a more well-defined interface. Feel free to improve it in any way you wish, but I'm leaving it as-is now.

That leaves us with the kmacro-tests failures. Android merge fall-out?

Message #14 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattias.engdegard <at> gmail.com>
To: Po Lu <luangruo <at> yahoo.com>
Cc: Emacs Bug Report <bug-gnu-emacs <at> gnu.org>
Subject: Re: dired-aux-tests and kmacro-tests failures on master
Date: Tue, 8 Aug 2023 12:42:42 +0200

Let's take the opportunity to fix the fileio-tests failure as well, apparently caused by 3fb2c174d3.

Message #17 received at 65143 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Mattias Engdegård <mattias.engdegard <at> gmail.com>
Cc: Po Lu <luangruo <at> yahoo.com>, 65143 <at> debbugs.gnu.org
Subject: Re: bug#65143: dired-aux-tests and kmacro-tests failures on master
Date: Fri, 15 Sep 2023 05:28:11 -0700

Mattias Engdegård <mattias.engdegard <at> gmail.com> writes:

> That leaves us with the kmacro-tests failures. Android merge fall-out?

Mattias Engdegård <mattias.engdegard <at> gmail.com> writes:

> Let's take the opportunity to fix the fileio-tests failure as well,
> apparently caused by 3fb2c174d3.

Are you still seeing these failures?  I'm seeing other test failures
intermittently, but not currently in these files.

Message #20 received at 65143 <at> debbugs.gnu.org (full text, mbox):

From: Po Lu <luangruo <at> yahoo.com>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: Mattias Engdegård <mattias.engdegard <at> gmail.com>,
 65143 <at> debbugs.gnu.org
Subject: Re: bug#65143: dired-aux-tests and kmacro-tests failures on master
Date: Fri, 15 Sep 2023 20:29:25 +0800

Stefan Kangas <stefankangas <at> gmail.com> writes:

> Mattias Engdegård <mattias.engdegard <at> gmail.com> writes:
>
>> That leaves us with the kmacro-tests failures. Android merge fall-out?
>
> Mattias Engdegård <mattias.engdegard <at> gmail.com> writes:
>
>> Let's take the opportunity to fix the fileio-tests failure as well,
>> apparently caused by 3fb2c174d3.
>
> Are you still seeing these failures?  I'm seeing other test failures
> intermittently, but not currently in these files.

I fixed the kmacro and fileio failures.

Message #25 received at 65143-done <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Po Lu <luangruo <at> yahoo.com>
Cc: Mattias Engdegård <mattias.engdegard <at> gmail.com>,
 65143-done <at> debbugs.gnu.org
Subject: Re: bug#65143: dired-aux-tests and kmacro-tests failures on master
Date: Fri, 15 Sep 2023 05:34:30 -0700

Po Lu <luangruo <at> yahoo.com> writes:

> I fixed the kmacro and fileio failures.

Thanks, I'm therefore closing this bug report.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.