dired-tests.el fails on MS-Windows

classic Classic list List threaded Threaded
22 messages Options
12
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

dired-tests.el fails on MS-Windows

Eli Zaretskii
The failed tests are shown below.  2 others failed originally, but I
fixed them.

Tino, please always add comments to the tests explaining the idea
behind all the tricks and juggling.  Otherwise, it is very hard to
understand why the test is doing what it's doing and how it is doing
that.  Same request for ediff-ptch-tests.el, which also fails for me.

Than ks.

dired-tests.log:

  Running 12 tests (2017-08-01 17:44:38+0300)
     passed   1/12  dired-autoload
  Marking matching files...
  Checking d:/gnu/git/emacs/trunk/test/bug22694/test
  1 matching file marked.
     passed   2/12  dired-test-bug22694
  Copy: 1 of 1
  Copy: 1 file
  Copy: 1 of 1
  Copy: 1 file
  Test dired-test-bug25609 backtrace:
    signal(ert-test-failed (((should (file-exists-p target)) :form (file
    ert-fail(((should (file-exists-p target)) :form (file-exists-p "c:/D
    (if (unwind-protect (setq value-17 (apply fn-15 args-16)) (setq form
    (let (form-description-19) (if (unwind-protect (setq value-17 (apply
    (let ((value-17 'ert-form-evaluation-aborted-18)) (let (form-descrip
    (let ((fn-15 (function file-exists-p)) (args-16 (list target))) (let
    (progn (let ((fn-15 (function file-exists-p)) (args-16 (list target)
    (unwind-protect (progn (let ((fn-15 (function file-exists-p)) (args-
    (let* ((from (make-temp-file "foo" 'dir)) (to (make-temp-file "bar"
    (closure (t) nil (let* ((from (make-temp-file "foo" 'dir)) (to (make
    ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
    ert-run-test(#s(ert-test :name dired-test-bug25609 :documentation "T
    ert-run-or-rerun-test(#s(ert--stats :selector t :tests [#s(ert-test
    ert-run-tests(t #f(compiled-function (event-type &rest event-args) #
    ert-run-tests-batch(nil)
    ert-run-tests-batch-and-exit(nil)
    eval((ert-run-tests-batch-and-exit nil))
    command-line-1(("-L" ";." "-l" "ert" "-l" "lisp/dired-tests.el" "--e
    command-line()
    normal-top-level()
  Test dired-test-bug25609 condition:
      (ert-test-failed
       ((should
         (file-exists-p target))
        :form
        (file-exists-p "c:/DOCUME~1/Zaretzky/LOCALS~1/Temp/bar6828Ler/foo6828WPJ")
        :value nil))
     FAILED   3/12  dired-test-bug25609
     passed   4/12  dired-test-bug27243-01
     passed   5/12  dired-test-bug27243-02
     passed   6/12  dired-test-bug27243-03
  Test dired-test-bug27631 backtrace:
    signal(error ("em-ls is not a currently loaded feature"))
    error("%s is not a currently loaded feature" "em-ls")
    unload-feature(em-ls force)
    (unwind-protect (progn (make-directory dir1) (make-directory dir2) (
    (let* ((dir (make-temp-file "bug27631" 'dir)) (dir1 (expand-file-nam
    (closure (t) nil (let* ((dir (make-temp-file "bug27631" 'dir)) (dir1
    ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
    ert-run-test(#s(ert-test :name dired-test-bug27631 :documentation "T
    ert-run-or-rerun-test(#s(ert--stats :selector t :tests [#s(ert-test
    ert-run-tests(t #f(compiled-function (event-type &rest event-args) #
    ert-run-tests-batch(nil)
    ert-run-tests-batch-and-exit(nil)
    eval((ert-run-tests-batch-and-exit nil))
    command-line-1(("-L" ";." "-l" "ert" "-l" "lisp/dired-tests.el" "--e
    command-line()
    normal-top-level()
  Test dired-test-bug27631 condition:
      (error "em-ls is not a currently loaded feature")
     FAILED   7/12  dired-test-bug27631
     passed   8/12  dired-test-bug27693
     failed   9/12  dired-test-bug27762
     passed  10/12  dired-test-bug27817
     passed  11/12  dired-test-bug27843
     passed  12/12  dired-test-bug7131

  Ran 12 tests, 10 results as expected, 2 unexpected (2017-08-01 17:44:46+0300)
  1 expected failures

  2 unexpected results:
     FAILED  dired-test-bug25609
     FAILED  dired-test-bug27631

ediff-ptch-tests.log:

  Running 2 tests (2017-08-01 18:21:40+0300)
     passed  1/2  ediff-ptch-test-bug25010
  Test ediff-ptch-test-bug26084 backtrace:
  Test ediff-ptch-test-bug26084 condition:
      (wrong-type-argument stringp nil)
     FAILED  2/2  ediff-ptch-test-bug26084

  Ran 2 tests, 1 results as expected, 1 unexpected (2017-08-01 18:21:40+0300)

  1 unexpected results:
     FAILED  ediff-ptch-test-bug26084


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: dired-tests.el fails on MS-Windows

Tino Calancha-2
Eli Zaretskii <[hidden email]> writes:

> The failed tests are shown below.  2 others failed originally, but I
> fixed them.
>
> Tino, please always add comments to the tests explaining the idea
> behind all the tricks and juggling.  Otherwise, it is very hard to
> understand why the test is doing what it's doing and how it is doing
> that.  Same request for ediff-ptch-tests.el, which also fails for me.
OK.  I agree they looks not very digestive tests.

> dired-tests.log:
>   Test dired-test-bug25609 condition:
>       (ert-test-failed
>        ((should
> (file-exists-p target))
> :form
> (file-exists-p "c:/DOCUME~1/Zaretzky/LOCALS~1/Temp/bar6828Ler/foo6828WPJ")
> :value nil))
Could you check the following?
emacs -Q
;; eval following form:
(require 'nadvice)
(let* ((from (make-temp-file "foo" 'dir))
       (to (make-temp-file "bar" 'dir))
       (target (expand-file-name (file-name-nondirectory from) to))
       (nested (expand-file-name (file-name-nondirectory from) target))
       (dired-dwim-target t)
       (dired-recursive-copies 'always) ; Don't prompt me.
       buffers)
  (advice-add 'dired-query ; Don't ask confirmation to overwrite a file.
              :override
              (lambda (_sym _prompt &rest _args) (setq dired-query t))
              '((name . "advice-dired-query")))
  (advice-add 'completing-read ; Just return init.
              :override
              (lambda (_prompt _coll &optional _pred _match init _hist _def _inherit _keymap)
                init)
              '((name . "advice-completing-read")))
  (push (dired to) buffers)
  (push (dired-other-window temporary-file-directory) buffers)
  (dired-goto-file from)
  (dired-do-copy)
  (dired-do-copy); Again.
  (unwind-protect
      (progn
        (list (file-exists-p target) (file-exists-p nested)))
    (dolist (buf buffers)
      (when (buffer-live-p buf) (kill-buffer buf)))
    (delete-directory from 'recursive)
    (delete-directory to 'recursive)
    (advice-remove 'dired-query "advice-dired-query")
    (advice-remove 'completing-read "advice-completing-read")))
;; The normal result is: '(t nil).  If you get '(nil t) means
;; the bug is not fixed in your platform.  Other values must be
;; wrong assumptions from my side that are not true in your box.


>   Test dired-test-bug27631 backtrace:
>     signal(error ("em-ls is not a currently loaded feature"))
>     error("%s is not a currently loaded feature" "em-ls")
>     unload-feature(em-ls force)
>     (unwind-protect (progn (make-directory dir1) (make-directory dir2) (
>     (let* ((dir (make-temp-file "bug27631" 'dir)) (dir1 (expand-file-nam
>     (closure (t) nil (let* ((dir (make-temp-file "bug27631" 'dir)) (dir1
>     ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
>     ert-run-test(#s(ert-test :name dired-test-bug27631 :documentation "T
>     ert-run-or-rerun-test(#s(ert--stats :selector t :tests [#s(ert-test
>     ert-run-tests(t #f(compiled-function (event-type &rest event-args) #
>     ert-run-tests-batch(nil)
>     ert-run-tests-batch-and-exit(nil)
>     eval((ert-run-tests-batch-and-exit nil))
>     command-line-1(("-L" ";." "-l" "ert" "-l" "lisp/dired-tests.el" "--e
>     command-line()
>     normal-top-level()
>   Test dired-test-bug27631 condition:
>       (error "em-ls is not a currently loaded feature")
I don't get this error, but the idea of require
those libs and unload them looks ugly.
We could move those parts to:
test/lisp/ls-lisp.el (yes i called wrongly: must be ls-lisp-tests.el)
test/lisp/eshell/eshell-tests.el
and then, we don't need to unload anything.

> ediff-ptch-tests.log:
>
>   Running 2 tests (2017-08-01 18:21:40+0300)
>      passed  1/2  ediff-ptch-test-bug25010
>   Test ediff-ptch-test-bug26084 backtrace:
>   Test ediff-ptch-test-bug26084 condition:
>       (wrong-type-argument stringp nil)
>      FAILED  2/2  ediff-ptch-test-bug26084
I think this test for Bug#26084 is more complicated than the
fix of the bug itself.  It has also problems because the different
idiosyncrasy respect to "-b" option for different versions of "patch".
Delete it?
Skipped it unless in a GNU system?

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: dired-tests.el fails on MS-Windows

Eli Zaretskii
> From: Tino Calancha <[hidden email]>
> Cc: [hidden email], [hidden email]
> Date: Wed, 02 Aug 2017 02:02:32 +0900
>
> > dired-tests.log:
> >   Test dired-test-bug25609 condition:
> >       (ert-test-failed
> >        ((should
> > (file-exists-p target))
> > :form
> > (file-exists-p "c:/DOCUME~1/Zaretzky/LOCALS~1/Temp/bar6828Ler/foo6828WPJ")
> > :value nil))
> Could you check the following?

I could, but I don't understand the purpose.  This form is almost
identical to what's in dired-tests.el, and I already established that
the failure is indeed because 'target' doesn't exist at that moment.
I just didn't dig deep enough to understand why, because I didn't
really understand what the code wants to do, e.g. why it calls
dired-do-copy twice, and more importantly why 'target' is supposed to
exist after all that.

What I see here is that at the point where file-exists-p is called,
there are two directories: /bla/blah/foNNNNNN and /bla/bla/barKKKKKK,
but not /bla/bla/fooNNNNN/barKKKKKK, as I think the code expects.

maybe if you could explain the idea behind the code I could think of a
reason why it doesn't work here.

> >   Test dired-test-bug27631 backtrace:
> >     signal(error ("em-ls is not a currently loaded feature"))
> >     error("%s is not a currently loaded feature" "em-ls")
> >     unload-feature(em-ls force)
> >     (unwind-protect (progn (make-directory dir1) (make-directory dir2) (
> >     (let* ((dir (make-temp-file "bug27631" 'dir)) (dir1 (expand-file-nam
> >     (closure (t) nil (let* ((dir (make-temp-file "bug27631" 'dir)) (dir1
> >     ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
> >     ert-run-test(#s(ert-test :name dired-test-bug27631 :documentation "T
> >     ert-run-or-rerun-test(#s(ert--stats :selector t :tests [#s(ert-test
> >     ert-run-tests(t #f(compiled-function (event-type &rest event-args) #
> >     ert-run-tests-batch(nil)
> >     ert-run-tests-batch-and-exit(nil)
> >     eval((ert-run-tests-batch-and-exit nil))
> >     command-line-1(("-L" ";." "-l" "ert" "-l" "lisp/dired-tests.el" "--e
> >     command-line()
> >     normal-top-level()
> >   Test dired-test-bug27631 condition:
> >       (error "em-ls is not a currently loaded feature")
> I don't get this error, but the idea of require
> those libs and unload them looks ugly.

I think we need to understand why the problem happens before we decide
how to proceed.

> > ediff-ptch-tests.log:
> >
> >   Running 2 tests (2017-08-01 18:21:40+0300)
> >      passed  1/2  ediff-ptch-test-bug25010
> >   Test ediff-ptch-test-bug26084 backtrace:
> >   Test ediff-ptch-test-bug26084 condition:
> >       (wrong-type-argument stringp nil)
> >      FAILED  2/2  ediff-ptch-test-bug26084
> I think this test for Bug#26084 is more complicated than the
> fix of the bug itself.  It has also problems because the different
> idiosyncrasy respect to "-b" option for different versions of "patch".
> Delete it?

I don't know.  What does it try to test?

> Skipped it unless in a GNU system?

Only if there's no better way.  The Patch invocation definitely needs
the --binary switch on Windows, though.  But the failure above is not
about that, it's about something else, because directory-files returns
an empty list.  Something prevents Patch from creating backup files.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: dired-tests.el fails on MS-Windows

Fabrice Popineau-4
In reply to this post by Eli Zaretskii
I don't get a failure on dired-test-bug25609 with windows 10 and running from a mingw64 bash.

I get a failure on  dired-test-bug27631 because "/bin/sh" is hardcoded in lisp/dired.el and the
place for sh.exe in msys2 is in /usr/bin .

I think that the various options for dired ls tests should be split in different test files (em-ls for example).
 
Fabrice


2017-08-01 17:22 GMT+02:00 Eli Zaretskii <[hidden email]>:
The failed tests are shown below.  2 others failed originally, but I
fixed them.

Tino, please always add comments to the tests explaining the idea
behind all the tricks and juggling.  Otherwise, it is very hard to
understand why the test is doing what it's doing and how it is doing
that.  Same request for ediff-ptch-tests.el, which also fails for me.

Than ks.

dired-tests.log:

  Running 12 tests (2017-08-01 17:44:38+0300)
     passed   1/12  dired-autoload
  Marking matching files...
  Checking d:/gnu/git/emacs/trunk/test/bug22694/test
  1 matching file marked.
     passed   2/12  dired-test-bug22694
  Copy: 1 of 1
  Copy: 1 file
  Copy: 1 of 1
  Copy: 1 file
  Test dired-test-bug25609 backtrace:
    signal(ert-test-failed (((should (file-exists-p target)) :form (file
    ert-fail(((should (file-exists-p target)) :form (file-exists-p "c:/D
    (if (unwind-protect (setq value-17 (apply fn-15 args-16)) (setq form
    (let (form-description-19) (if (unwind-protect (setq value-17 (apply
    (let ((value-17 'ert-form-evaluation-aborted-18)) (let (form-descrip
    (let ((fn-15 (function file-exists-p)) (args-16 (list target))) (let
    (progn (let ((fn-15 (function file-exists-p)) (args-16 (list target)
    (unwind-protect (progn (let ((fn-15 (function file-exists-p)) (args-
    (let* ((from (make-temp-file "foo" 'dir)) (to (make-temp-file "bar"
    (closure (t) nil (let* ((from (make-temp-file "foo" 'dir)) (to (make
    ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
    ert-run-test(#s(ert-test :name dired-test-bug25609 :documentation "T
    ert-run-or-rerun-test(#s(ert--stats :selector t :tests [#s(ert-test
    ert-run-tests(t #f(compiled-function (event-type &rest event-args) #
    ert-run-tests-batch(nil)
    ert-run-tests-batch-and-exit(nil)
    eval((ert-run-tests-batch-and-exit nil))
    command-line-1(("-L" ";." "-l" "ert" "-l" "lisp/dired-tests.el" "--e
    command-line()
    normal-top-level()
  Test dired-test-bug25609 condition:
      (ert-test-failed
       ((should
         (file-exists-p target))
        :form
        (file-exists-p "c:/DOCUME~1/Zaretzky/LOCALS~1/Temp/bar6828Ler/foo6828WPJ")
        :value nil))
     FAILED   3/12  dired-test-bug25609
     passed   4/12  dired-test-bug27243-01
     passed   5/12  dired-test-bug27243-02
     passed   6/12  dired-test-bug27243-03
  Test dired-test-bug27631 backtrace:
    signal(error ("em-ls is not a currently loaded feature"))
    error("%s is not a currently loaded feature" "em-ls")
    unload-feature(em-ls force)
    (unwind-protect (progn (make-directory dir1) (make-directory dir2) (
    (let* ((dir (make-temp-file "bug27631" 'dir)) (dir1 (expand-file-nam
    (closure (t) nil (let* ((dir (make-temp-file "bug27631" 'dir)) (dir1
    ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
    ert-run-test(#s(ert-test :name dired-test-bug27631 :documentation "T
    ert-run-or-rerun-test(#s(ert--stats :selector t :tests [#s(ert-test
    ert-run-tests(t #f(compiled-function (event-type &rest event-args) #
    ert-run-tests-batch(nil)
    ert-run-tests-batch-and-exit(nil)
    eval((ert-run-tests-batch-and-exit nil))
    command-line-1(("-L" ";." "-l" "ert" "-l" "lisp/dired-tests.el" "--e
    command-line()
    normal-top-level()
  Test dired-test-bug27631 condition:
      (error "em-ls is not a currently loaded feature")
     FAILED   7/12  dired-test-bug27631
     passed   8/12  dired-test-bug27693
     failed   9/12  dired-test-bug27762
     passed  10/12  dired-test-bug27817
     passed  11/12  dired-test-bug27843
     passed  12/12  dired-test-bug7131

  Ran 12 tests, 10 results as expected, 2 unexpected (2017-08-01 17:44:46+0300)
  1 expected failures

  2 unexpected results:
     FAILED  dired-test-bug25609
     FAILED  dired-test-bug27631

ediff-ptch-tests.log:

  Running 2 tests (2017-08-01 18:21:40+0300)
     passed  1/2  ediff-ptch-test-bug25010
  Test ediff-ptch-test-bug26084 backtrace:
  Test ediff-ptch-test-bug26084 condition:
      (wrong-type-argument stringp nil)
     FAILED  2/2  ediff-ptch-test-bug26084

  Ran 2 tests, 1 results as expected, 1 unexpected (2017-08-01 18:21:40+0300)

  1 unexpected results:
     FAILED  ediff-ptch-test-bug26084





--
Fabrice Popineau
-----------------------------
CentraleSupelec
Département Informatique
3, rue Joliot Curie
91192 Gif/Yvette Cedex
Tel direct : +33 (0) 169851950
Standard : +33 (0) 169851212
------------------------------

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: dired-tests.el fails on MS-Windows

Fabrice Popineau-3
In reply to this post by Eli Zaretskii


2017-08-01 21:04 GMT+02:00 Eli Zaretskii <[hidden email]>:

Only if there's no better way.  The Patch invocation definitely needs
the --binary switch on Windows, though.  But the failure above is not
about that, it's about something else, because directory-files returns
an empty list.  Something prevents Patch from creating backup files.


When I add the '--binary' option to patch, the test passes.
Again, windows 10, mingw64.



--
Fabrice
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: dired-tests.el fails on MS-Windows

Tino Calancha-2
In reply to this post by Fabrice Popineau-4


On Tue, 1 Aug 2017, Fabrice Popineau wrote:

> I don't get a failure on dired-test-bug25609 with windows 10 and running from a mingw64 bash.
> I get a failure on  dired-test-bug27631 because "/bin/sh" is hardcoded in lisp/dired.el and the
> place for sh.exe in msys2 is in /usr/bin .
Thank you for the comments and suggestions!

I should used 'executable-find' for a local connection.  For a
tramp connection i don't know how to get the 'sh' location in the
remote host: i just kept '/bin/sh' for them.
Michael?

--8<-----------------------------cut here---------------start------------->8---

commit 76f1ea53c7469f7d4c5b9007633642124ae88c62
Author: Tino Calancha <[hidden email]>
Date:   Wed Aug 2 12:27:42 2017 +0900

     Don't assume /bin/sh as the 'sh' location in the local host

     * lisp/dired.el (dired-insert-directory): Use executable-find in
     a local host.

diff --git a/lisp/dired.el b/lisp/dired.el
index 4f8f615a34..a0e1fe185d 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -1281,7 +1281,8 @@ dired-insert-directory
                 (unless
                     (zerop
                      (process-file
-                     "/bin/sh" nil (current-buffer) nil "-c" script))
+                     (or (and (file-remote-p default-directory) "/bin/sh") (executable-find "sh"))
+                     nil (current-buffer) nil "-c" script))
                   (user-error
                    "%s: No files matching wildcard" (cdr dir-wildcard)))
                 (insert-directory-clean (point) switches)))

--8<-----------------------------cut here---------------end--------------->8---
In GNU Emacs 26.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.11)
  of 2017-08-02
Repository revision: 0fd6de9cb444d6cc553ea67815ccfb7a923012a2
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: dired-tests.el fails on MS-Windows

Michael Albinus
Tino Calancha <[hidden email]> writes:

Hi Tino.

> I should used 'executable-find' for a local connection.  For a
> tramp connection i don't know how to get the 'sh' location in the
> remote host: i just kept '/bin/sh' for them.
> Michael?

"/bin/sh" is OK. The feature works anyway only for methods defined in
tramp-sh.el.

Alternatively, one could take the value of `explicit-shell-file-name',
which is prepared for connection-local variables. But I doubt, that many
people use it this way already.

Best regards, Michael.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: dired-tests.el fails on MS-Windows

Tino Calancha-2
In reply to this post by Fabrice Popineau-3


On Tue, 1 Aug 2017, Fabrice Popineau wrote:

>
>
> 2017-08-01 21:04 GMT+02:00 Eli Zaretskii <[hidden email]>:
>
>       Only if there's no better way.  The Patch invocation definitely needs
>       the --binary switch on Windows, though.  But the failure above is not
>       about that, it's about something else, because directory-files returns
>       an empty list.  Something prevents Patch from creating backup files.
>
>
> When I add the '--binary' option to patch, the test passes.
> Again, windows 10, mingw64.
Fabrice, Eli
does the following work in your environments?

--8<-----------------------------cut here---------------start------------->8---
commit 16fd5bf68538240b7a601e0975bdd92f0521b7e5
Author: Tino Calancha <[hidden email]>
Date:   Wed Aug 2 15:37:11 2017 +0900

     Fix an ediff test which fails in MS-Windows

     https://lists.gnu.org/archive/html/emacs-devel/2017-08/msg00018.html
     * test/lisp/vc/ediff-ptch-tests.el (ediff-ptch-test-bug26084):
     Add comments to explain the test logic.
     Pass '--binary' option to 'patch' program in windows environments.
     Check explicitely that a backup is created before compare file contents.

diff --git a/test/lisp/vc/ediff-ptch-tests.el b/test/lisp/vc/ediff-ptch-tests.el
index 387786ced0..6fbc1b0a8b 100644
--- a/test/lisp/vc/ediff-ptch-tests.el
+++ b/test/lisp/vc/ediff-ptch-tests.el
@@ -66,41 +66,55 @@
        (write-region nil nil bar nil 'silent))
      (call-process git-program nil `(:file ,patch) nil "diff")
      (call-process git-program nil nil nil "reset" "--hard" "HEAD")
+    ;; Visit the diff file i.e., patch; extract from it the parts
+    ;; affecting just each of the files: store in patch-bar the part
+    ;; affecting 'bar', and in patch-qux the part affecting 'qux'.
      (find-file patch)
      (unwind-protect
          (let* ((info
                  (progn (ediff-map-patch-buffer (current-buffer)) ediff-patch-map))
-               (patch1
+               (patch-bar
                  (buffer-substring-no-properties
                   (car (nth 3 (car info)))
                   (car (nth 4 (car info)))))
-               (patch2
+               (patch-qux
                  (buffer-substring-no-properties
                   (car (nth 3 (cadr info)))
                   (car (nth 4 (cadr info))))))
            ;; Apply both patches.
-          (dolist (x (list (cons patch1 bar) (cons patch2 qux)))
+          (dolist (x (list (cons patch-bar bar) (cons patch-qux qux)))
              (with-temp-buffer
-              (insert (car x))
-              (call-process-region (point-min)
-                                   (point-max)
-                                   ediff-patch-program
-                                   nil nil nil
-                                   "-b" (cdr x))))
-          ;; Check backup files were saved correctly.
+              ;; Some windows variants require the option '--binary'
+              ;; in order to 'patch' create backup files.
+              (let ((opts (format "--backup%s"
+                                  (if (memq system-type '(windows-nt ms-dos))
+                                      " --binary" ""))))
+                (insert (car x))
+                (call-process-region (point-min)
+                                     (point-max)
+                                     ediff-patch-program
+                                     nil nil nil
+                                     opts (cdr x)))))
+          ;; Check backup files were saved correctly; in Bug#26084 some
+          ;; of the backup files are overwritten with the actual content
+          ;; of the updated file.  To ensure that the bug is fixed we just
+          ;; need to check that every backup file produced has different
+          ;; content that the current updated file.
            (dolist (x (list qux bar))
              (let ((backup
                     (car
                      (directory-files
                       tmpdir 'full
                       (concat (file-name-nondirectory x) ".")))))
-              (should-not
-               (string= (with-temp-buffer
-                          (insert-file-contents x)
-                          (buffer-string))
-                        (with-temp-buffer
-                          (insert-file-contents backup)
-                          (buffer-string))))))
+              ;; Compare files only if the backup has being created.
+              (when backup
+                (should-not
+                 (string= (with-temp-buffer
+                            (insert-file-contents x)
+                            (buffer-string))
+                          (with-temp-buffer
+                            (insert-file-contents backup)
+                            (buffer-string)))))))
            (delete-directory tmpdir 'recursive)
            (delete-file patch)))))

--8<-----------------------------cut here---------------end--------------->8---
In GNU Emacs 26.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.11)
  of 2017-08-02 built
Repository revision: 0fd6de9cb444d6cc553ea67815ccfb7a923012a2
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: dired-tests.el fails on MS-Windows

Tino Calancha-2
In reply to this post by Michael Albinus


On Wed, 2 Aug 2017, Michael Albinus wrote:

> Tino Calancha <[hidden email]> writes:
>
> Hi Tino.
>
>> I should used 'executable-find' for a local connection.  For a
>> tramp connection i don't know how to get the 'sh' location in the
>> remote host: i just kept '/bin/sh' for them.
>> Michael?
>
> "/bin/sh" is OK. The feature works anyway only for methods defined in
> tramp-sh.el.
>
> Alternatively, one could take the value of `explicit-shell-file-name',
> which is prepared for connection-local variables. But I doubt, that many
> people use it this way already.
Thank you.  I pushed in e82c4f56e6
(Don't assume /bin/sh as the 'sh' location in the local host)
a fix: in the local connection check first for a non-nil
value of explicit-shell-file-name to exists; otherwise it
uses (executable-find "sh").

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: dired-tests.el fails on MS-Windows

Tino Calancha-2
In reply to this post by Eli Zaretskii


On Tue, 1 Aug 2017, Eli Zaretskii wrote:

>> From: Tino Calancha <[hidden email]>
>> Cc: [hidden email], [hidden email]
>> Date: Wed, 02 Aug 2017 02:02:32 +0900
>>
>>> dired-tests.log:
>>>   Test dired-test-bug25609 condition:
>>>       (ert-test-failed
>>>        ((should
>>> (file-exists-p target))
>>> :form
>>> (file-exists-p "c:/DOCUME~1/Zaretzky/LOCALS~1/Temp/bar6828Ler/foo6828WPJ")
>>> :value nil))
>> Could you check the following?
>
> I could, but I don't understand the purpose.  This form is almost
> identical to what's in dired-tests.el, and I already established that
> the failure is indeed because 'target' doesn't exist at that moment.
> I just didn't dig deep enough to understand why, because I didn't
> really understand what the code wants to do, e.g. why it calls
> dired-do-copy twice, and more importantly why 'target' is supposed to
> exist after all that.
>
> What I see here is that at the point where file-exists-p is called,
> there are two directories: /bla/blah/foNNNNNN and /bla/bla/barKKKKKK,
> but not /bla/bla/fooNNNNN/barKKKKKK, as I think the code expects.
>
> maybe if you could explain the idea behind the code I could think of a
> reason why it doesn't work here.
I added more comments and sanity checks in commit
db5d38ddb0de83d8f920b7a128fe3fd5156fdf85
(Fix 2 tests that fail in MS-Windows)
Does it work now in Windows?

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: dired-tests.el fails on MS-Windows

Eli Zaretskii
> From: Tino Calancha <[hidden email]>
> Date: Fri, 4 Aug 2017 14:21:01 +0900 (JST)
> cc: Tino Calancha <[hidden email]>,
>     Emacs developers <[hidden email]>
>
> > maybe if you could explain the idea behind the code I could think of a
> > reason why it doesn't work here.
> I added more comments and sanity checks in commit
> db5d38ddb0de83d8f920b7a128fe3fd5156fdf85
> (Fix 2 tests that fail in MS-Windows)
> Does it work now in Windows?

It didn't, but given the comments I've now succeeded to understand the
idea of dired-test-bug25609, and fixed it.

dired-test-bug27631 still fails, and it fails because of this:

          (setq buf (dired (expand-file-name "dir*/*.txt" dir)))

ls-lisp signals an error here:

   wrong-type-argument listp "dir*/*.txt"

Didn't you add a feature lately that should support this in ls-lisp?
I guess that feature needs to be turned on for this test to pass on
Windows.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: dired-tests.el fails on MS-Windows

Eli Zaretskii
In reply to this post by Tino Calancha-2
> From: Tino Calancha <[hidden email]>
> Date: Wed, 2 Aug 2017 15:44:59 +0900 (JST)
> cc: Eli Zaretskii <[hidden email]>, Tino Calancha <[hidden email]>,
>     Emacs developers <[hidden email]>
>
> > 2017-08-01 21:04 GMT+02:00 Eli Zaretskii <[hidden email]>:
> >
> >       Only if there's no better way.  The Patch invocation definitely needs
> >       the --binary switch on Windows, though.  But the failure above is not
> >       about that, it's about something else, because directory-files returns
> >       an empty list.  Something prevents Patch from creating backup files.
> >
> >
> > When I add the '--binary' option to patch, the test passes.
> > Again, windows 10, mingw64.
> Fabrice, Eli
> does the following work in your environments?

Yes, it passes now.

Thanks!

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: dired-tests.el fails on MS-Windows

Fabrice Popineau-3
I shall say : it almost passes on my side.

I had to apply the following :

diff --git a/test/lisp/dired-tests.el b/test/lisp/dired-tests.el
index 02dbf263b9..88cdbef6ac 100644
--- a/test/lisp/dired-tests.el
+++ b/test/lisp/dired-tests.el
@@ -282,7 +282,7 @@ dired-dwim-target
           (make-directory dir2)
           (with-temp-file (expand-file-name "a.txt" dir1))
           (with-temp-file (expand-file-name "b.txt" dir2))
-          (setq buf (dired (expand-file-name "dir*/*.txt" dir)))
+          (setq buf (dired (cons dir (file-expand-wildcards "dir*/*.txt"))))
           (dired-toggle-marks)
           (should (cdr (dired-get-marked-files))))
       (delete-directory dir 'recursive)
diff --git a/test/lisp/eshell/em-ls-tests.el b/test/lisp/eshell/em-ls-tests.el
index 71a555d1ea..cc0e68c47d 100644
--- a/test/lisp/eshell/em-ls-tests.el
+++ b/test/lisp/eshell/em-ls-tests.el
@@ -42,7 +42,7 @@
           (make-directory dir2)
           (with-temp-file (expand-file-name "a.txt" dir1))
           (with-temp-file (expand-file-name "b.txt" dir2))
-          (setq buf (dired (expand-file-name "dir*/*.txt" dir)))
+          (setq buf (dired (cons dir (file-expand-wildcards "dir*/*.txt"))))
           (dired-toggle-marks)
           (should (cdr (dired-get-marked-files))))
       (customize-set-variable 'eshell-ls-use-in-dired orig)
diff --git a/test/lisp/ls-lisp-tests.el b/test/lisp/ls-lisp-tests.el
index d24b30e5f2..77a02c88dd 100644
--- a/test/lisp/ls-lisp-tests.el
+++ b/test/lisp/ls-lisp-tests.el
@@ -69,7 +69,7 @@
           (make-directory dir2)
           (with-temp-file (expand-file-name "a.txt" dir1))
           (with-temp-file (expand-file-name "b.txt" dir2))
-          (setq buf (dired (expand-file-name "dir*/*.txt" dir)))
+          (setq buf (dired (cons dir (file-expand-wildcards "dir*/*.txt"))))
           (dired-toggle-marks)
           (should (cdr (dired-get-marked-files))))
       (delete-directory dir 'recursive)

Am I wrong thinking that `expand-file-name' is not supposed to expand "dir*/*.txt"?

Fabrice


2017-08-04 15:18 GMT+02:00 Eli Zaretskii <[hidden email]>:
> From: Tino Calancha <[hidden email]>
> Date: Wed, 2 Aug 2017 15:44:59 +0900 (JST)
> cc: Eli Zaretskii <[hidden email]>, Tino Calancha <[hidden email]>,
>     Emacs developers <[hidden email]>
>
> > 2017-08-01 21:04 GMT+02:00 Eli Zaretskii <[hidden email]>:
> >
> >       Only if there's no better way.  The Patch invocation definitely needs
> >       the --binary switch on Windows, though.  But the failure above is not
> >       about that, it's about something else, because directory-files returns
> >       an empty list.  Something prevents Patch from creating backup files.
> >
> >
> > When I add the '--binary' option to patch, the test passes.
> > Again, windows 10, mingw64.
> Fabrice, Eli
> does the following work in your environments?

Yes, it passes now.

Thanks!

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: dired-tests.el fails on MS-Windows

Tino Calancha-2
In reply to this post by Eli Zaretskii


On Fri, 4 Aug 2017, Eli Zaretskii wrote:

>> Does it work now in Windows?
>
> It didn't, but given the comments I've now succeeded to understand the
> idea of dired-test-bug25609, and fixed it.
Great! Thank you.

> dired-test-bug27631 still fails, and it fails because of this:
>
>          (setq buf (dired (expand-file-name "dir*/*.txt" dir)))
>
> ls-lisp signals an error here:
>
>   wrong-type-argument listp "dir*/*.txt"
>
> Didn't you add a feature lately that should support this in ls-lisp?
> I guess that feature needs to be turned on for this test to pass on
> Windows.
Yeah, that test must be skipped when Dired is using 'ls' emulation.  We
have tests for the same bug in ls-lisp-tests.el and em-ls-tests.el.
I've just pushed a fix.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: dired-tests.el fails on MS-Windows

Tino Calancha-2
In reply to this post by Fabrice Popineau-3


On Fri, 4 Aug 2017, Fabrice Popineau wrote:

>            (with-temp-file (expand-file-name "b.txt" dir2))
> -          (setq buf (dired (expand-file-name "dir*/*.txt" dir)))
> +          (setq buf (dired (cons dir (file-expand-wildcards "dir*/*.txt"))))
>            (dired-toggle-marks)
>            (should (cdr (dired-get-marked-files))))
>        (delete-directory dir 'recursive)
>
> Am I wrong thinking that `expand-file-name' is not supposed to expand "dir*/*.txt"?
Well we are using expand for 2 different things.

1) Expand filename with directory:
(expand-file-name "lis*/*file" source-directory)
=> "/home/calancha/soft/emacs-master/lis*/*file"
;; Like a concatenation of file + dir.

2) Expand shell wildcards.
(dired (expand-file-name "lis*/*file" source-directory))

Shows a dired buffer with 3 files:
lispref/Makefile
lispintro/Makefile
lisp/Makefile
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: dired-tests.el fails on MS-Windows

Eli Zaretskii
In reply to this post by Fabrice Popineau-3
> From: Fabrice Popineau <[hidden email]>
> Date: Fri, 4 Aug 2017 15:30:49 +0200
> Cc: Tino Calancha <[hidden email]>, Emacs developers <[hidden email]>
>
> Am I wrong thinking that `expand-file-name' is not supposed to expand "dir*/*.txt"?

You are not wrong.  But the problem is not in expand-file-name: it
wasn't supposed to expand the wildcards.  The problem is in dired:
when it uses the 'ls' command, the wildcards are expanded by the
shell.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: dired-tests.el fails on MS-Windows

Fabrice Popineau-3
In reply to this post by Tino Calancha-2


2017-08-04 15:44 GMT+02:00 Tino Calancha <[hidden email]>:


On Fri, 4 Aug 2017, Fabrice Popineau wrote:

           (with-temp-file (expand-file-name "b.txt" dir2))
-          (setq buf (dired (expand-file-name "dir*/*.txt" dir)))
+          (setq buf (dired (cons dir (file-expand-wildcards "dir*/*.txt"))))
           (dired-toggle-marks)
           (should (cdr (dired-get-marked-files))))
       (delete-directory dir 'recursive)

Am I wrong thinking that `expand-file-name' is not supposed to expand "dir*/*.txt"?
Well we are using expand for 2 different things.

2) Expand shell wildcards.
(dired (expand-file-name "lis*/*file" source-directory))

This doesn't work here.

I have :

c:/tmp/dir1/foo.txt
c:/tmp/dir2/bar.txt
 
and:

(expand-file-name "dir*/*.txt" "c:/tmp/")
"c:/tmp/dir*/*.txt"

Moreover, as far as I can read it, `expand-file-name' documentation says nothing about 
expanding wildcards.

And last, `dired' maybe fed with a list, but the first argument needs to be the directory
and the next ones, the files you want in the dired buffer, so I doubt that `dired' will accomodate
the return value(s?) of `expand-file-name'?

-> In other words: I don't understand how this should work :-)



Fabrice


Shows a dired buffer with 3 files:
lispref/Makefile
lispintro/Makefile
lisp/Makefile

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: dired-tests.el fails on MS-Windows

Fabrice Popineau-3
In reply to this post by Eli Zaretskii


2017-08-04 15:14 GMT+02:00 Eli Zaretskii <[hidden email]>:
> From: Tino Calancha <[hidden email]>
> Date: Fri, 4 Aug 2017 14:21:01 +0900 (JST)
> cc: Tino Calancha <[hidden email]>,
>     Emacs developers <[hidden email]>
>
> > maybe if you could explain the idea behind the code I could think of a
> > reason why it doesn't work here.
> I added more comments and sanity checks in commit
> db5d38ddb0de83d8f920b7a128fe3fd5156fdf85
> (Fix 2 tests that fail in MS-Windows)
> Does it work now in Windows?

It didn't, but given the comments I've now succeeded to understand the
idea of dired-test-bug25609, and fixed it.

dired-test-bug27631 still fails, and it fails because of this:

          (setq buf (dired (expand-file-name "dir*/*.txt" dir)))

ls-lisp signals an error here:

   wrong-type-argument listp "dir*/*.txt"

Didn't you add a feature lately that should support this in ls-lisp?
I guess that feature needs to be turned on for this test to pass on
Windows.


Actually, the problem seems to be in the `insert-directory-wildcard-in-dir-p' function
which wrongly splits "c:/tmp/dir*/*.txt" in ("c:/tmp/" . "dir*/*.txt") instead of
("c:/tmp/dir*/" . "*.txt")


--
Fabrice
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: dired-tests.el fails on MS-Windows

Fabrice Popineau-3


2017-08-04 16:23 GMT+02:00 Fabrice Popineau <[hidden email]>:


Actually, the problem seems to be in the `insert-directory-wildcard-in-dir-p' function
which wrongly splits "c:/tmp/dir*/*.txt" in ("c:/tmp/" . "dir*/*.txt") instead of
("c:/tmp/dir*/" . "*.txt")

Forget this (wrong) diagnostic.

The culprit is actually 

(let ((default-directory "c:/tmp/"))
  (eshell-extended-glob "dir*/*.txt"))
"dir*/*.txt" 

which fails to expand the wildcards (when `file-expand-wildcards' succeeds).


--
Fabrice

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: dired-tests.el fails on MS-Windows

Tino Calancha-2


On Fri, 4 Aug 2017, Fabrice Popineau wrote:

>
>
> 2017-08-04 16:23 GMT+02:00 Fabrice Popineau <[hidden email]>:
>
>
> Actually, the problem seems to be in the `insert-directory-wildcard-in-dir-p' function
> which wrongly splits "c:/tmp/dir*/*.txt" in ("c:/tmp/" . "dir*/*.txt") instead of
> ("c:/tmp/dir*/" . "*.txt")
>
> Forget this (wrong) diagnostic.
>
> The culprit is actually 
>
> (let ((default-directory "c:/tmp/"))
>   (eshell-extended-glob "dir*/*.txt"))
> "dir*/*.txt" 
>
> which fails to expand the wildcards (when `file-expand-wildcards' succeeds).
Thank you Fabrice,
that's interesing.  I am just wondering if `eshell-extended-glob' gets
confused with the Windows path, i mean, the disk name 'c:' in front.

Could you check if the following works?
M-x eshell RET
cd "c:/tmp"
ls -l dir*/*.txt

I am also curious if:
M-: (equal temporary-file-directory "c:/tmp/") RET
=> t

Thank you,
Tino
12
Loading...