unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Question about test failure on Hydra
@ 2017-07-30 15:50 Stephen Berman
  2017-07-30 15:59 ` Eli Zaretskii
  2017-08-03 19:41 ` tagging ERT tests with bug numbers (was: Question about test failure on Hydra) Ted Zlatanov
  0 siblings, 2 replies; 20+ messages in thread
From: Stephen Berman @ 2017-07-30 15:50 UTC (permalink / raw)
  To: emacs-devel

Part of the Dired tests for bug#27243 continues to fail intermittently
on Hydra and I see in the logfiles why it fails, but I don't understand
how the trigger for the failure arises.  Here's the test code (for the
first of the bug cases; the second case has almost the same code and
fails in the same way) up to the point of failure (the should sexp):

(ert-deftest dired-test-bug27243-01 ()
  "Test for https://debbugs.gnu.org/cgi/bugreport.cgi?bug=27243#5 ."
  (let ((test-dir (make-temp-file "test-dir-" t))
        (dired-auto-revert-buffer t) buffers)
    (with-current-buffer (find-file-noselect test-dir)
      (make-directory "test-subdir"))
    (push (dired test-dir) buffers)
    (unwind-protect
        (let ((buf (current-buffer))
              (pt1 (point))
              (test-file (concat (file-name-as-directory "test-subdir")
                                 "test-file")))
          (write-region "Test" nil test-file nil 'silent nil 'excl)
          ;; Sanity check: point should now be on the subdirectory.
          (should (equal (dired-file-name-at-point)
                         (concat (file-name-as-directory test-dir)
                                 (file-name-as-directory "test-subdir"))))

When it fails, Hydra show dired-file-name-at-point returning e.g. this
(it's a temporary directory so the name is different on each test run):

"/build/test-dir-26691mMc/../"

while the concat sexp it's being compared with returns (e.g.) this:

"/build/test-dir-26691mMc/test-subdir/"

IIUC dired-file-name-at-point can only return the above result if point
is on the ".." entry in Dired.  But the sexp (dired test-dir) should
(and in all my test runs does) put point on the first "nontrivial" file
(i.e. it skips "." and "..").  I don't see how the above code can lead
to the failure Hydra reports.  And again, it doesn't always fail in
Hydra.  Does anyone have any idea what's going on here and what to do
about it?

Steve Berman



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Question about test failure on Hydra
  2017-07-30 15:50 Question about test failure on Hydra Stephen Berman
@ 2017-07-30 15:59 ` Eli Zaretskii
  2017-07-30 20:23   ` Stephen Berman
  2017-08-03 19:41 ` tagging ERT tests with bug numbers (was: Question about test failure on Hydra) Ted Zlatanov
  1 sibling, 1 reply; 20+ messages in thread
From: Eli Zaretskii @ 2017-07-30 15:59 UTC (permalink / raw)
  To: Stephen Berman; +Cc: emacs-devel

> From: Stephen Berman <stephen.berman@gmx.net>
> Date: Sun, 30 Jul 2017 17:50:38 +0200
> 
> (ert-deftest dired-test-bug27243-01 ()
>   "Test for https://debbugs.gnu.org/cgi/bugreport.cgi?bug=27243#5 ."
>   (let ((test-dir (make-temp-file "test-dir-" t))
>         (dired-auto-revert-buffer t) buffers)
>     (with-current-buffer (find-file-noselect test-dir)
>       (make-directory "test-subdir"))
>     (push (dired test-dir) buffers)
>     (unwind-protect
>         (let ((buf (current-buffer))
>               (pt1 (point))
>               (test-file (concat (file-name-as-directory "test-subdir")
>                                  "test-file")))
>           (write-region "Test" nil test-file nil 'silent nil 'excl)
>           ;; Sanity check: point should now be on the subdirectory.
>           (should (equal (dired-file-name-at-point)
>                          (concat (file-name-as-directory test-dir)
>                                  (file-name-as-directory "test-subdir"))))
> 
> When it fails, Hydra show dired-file-name-at-point returning e.g. this
> (it's a temporary directory so the name is different on each test run):
> 
> "/build/test-dir-26691mMc/../"
> 
> while the concat sexp it's being compared with returns (e.g.) this:
> 
> "/build/test-dir-26691mMc/test-subdir/"
> 
> IIUC dired-file-name-at-point can only return the above result if point
> is on the ".." entry in Dired.  But the sexp (dired test-dir) should
> (and in all my test runs does) put point on the first "nontrivial" file
> (i.e. it skips "." and "..").  I don't see how the above code can lead
> to the failure Hydra reports.  And again, it doesn't always fail in
> Hydra.  Does anyone have any idea what's going on here and what to do
> about it?

Doesn't dired-auto-revert-buffer by default use some kind of file
notifications?  If so, this could be some race between the
notification coming in and the rest.  You could introduce a wait to
try to solve that, or maybe try turning off file notifications.



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Question about test failure on Hydra
  2017-07-30 15:59 ` Eli Zaretskii
@ 2017-07-30 20:23   ` Stephen Berman
  2017-07-31  3:26     ` Eli Zaretskii
  0 siblings, 1 reply; 20+ messages in thread
From: Stephen Berman @ 2017-07-30 20:23 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

On Sun, 30 Jul 2017 18:59:34 +0300 Eli Zaretskii <eliz@gnu.org> wrote:

> Doesn't dired-auto-revert-buffer by default use some kind of file
> notifications?  If so, this could be some race between the
> notification coming in and the rest.  You could introduce a wait to
> try to solve that, or maybe try turning off file notifications.

I stepped through dired-revert and followed the call chain to
insert-directory but didn't see anything that seemed related to file
notifications.  But I'm not familiar with file notifications in Emacs.
Can you point to where file notifications come into play here, or can
you tell me how I can find out if they are involved?

Steve Berman



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Question about test failure on Hydra
  2017-07-30 20:23   ` Stephen Berman
@ 2017-07-31  3:26     ` Eli Zaretskii
  2017-07-31  9:20       ` Stephen Berman
  0 siblings, 1 reply; 20+ messages in thread
From: Eli Zaretskii @ 2017-07-31  3:26 UTC (permalink / raw)
  To: Stephen Berman; +Cc: emacs-devel

> From: Stephen Berman <stephen.berman@gmx.net>
> Cc: emacs-devel@gnu.org
> Date: Sun, 30 Jul 2017 22:23:32 +0200
> 
> On Sun, 30 Jul 2017 18:59:34 +0300 Eli Zaretskii <eliz@gnu.org> wrote:
> 
> > Doesn't dired-auto-revert-buffer by default use some kind of file
> > notifications?  If so, this could be some race between the
> > notification coming in and the rest.  You could introduce a wait to
> > try to solve that, or maybe try turning off file notifications.
> 
> I stepped through dired-revert and followed the call chain to
> insert-directory but didn't see anything that seemed related to file
> notifications.

No, you are right, this option has nothing to do with file
notifications: we always revert Dired buffers "by hand", whenever one
of them is visited.

So, back to the original issue:

> But the sexp (dired test-dir) should (and in all my test runs does)
> put point on the first "nontrivial" file (i.e. it skips "." and
> "..").

In your case, does (dired test-dir) reads the directory anew, or does
it revert an existing Dired buffer?  I think the place to look for the
problem depends on the answer to this question.



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Question about test failure on Hydra
  2017-07-31  3:26     ` Eli Zaretskii
@ 2017-07-31  9:20       ` Stephen Berman
  2017-08-01  3:12         ` Eli Zaretskii
  2017-08-01  4:39         ` Tino Calancha
  0 siblings, 2 replies; 20+ messages in thread
From: Stephen Berman @ 2017-07-31  9:20 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

On Mon, 31 Jul 2017 06:26:55 +0300 Eli Zaretskii <eliz@gnu.org> wrote:

>> But the sexp (dired test-dir) should (and in all my test runs does)
>> put point on the first "nontrivial" file (i.e. it skips "." and
>> "..").
>
> In your case, does (dired test-dir) reads the directory anew, or does
> it revert an existing Dired buffer?  I think the place to look for the
> problem depends on the answer to this question.

(dired test-dir) reverts an existing Dired buffer, because of setting
dired-auto-revert-buffer to t.  (When I step through the code, it is
after reverting that point moves to the subdirectory line, which the
should make "sanity check" true (and does, both when I step through the
code and just run the test in any way).  When I comment out the
dired-auto-revert-buffer line, then point stays at point-min, which
makes the sanity check fail (dired-file-name-at-point returns nil),
though not in the way Hydra reports (it shows point being on the ".."
entry).)

Steve Berman



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Question about test failure on Hydra
  2017-07-31  9:20       ` Stephen Berman
@ 2017-08-01  3:12         ` Eli Zaretskii
  2017-08-01  9:57           ` Stephen Berman
  2017-08-01  4:39         ` Tino Calancha
  1 sibling, 1 reply; 20+ messages in thread
From: Eli Zaretskii @ 2017-08-01  3:12 UTC (permalink / raw)
  To: Stephen Berman; +Cc: emacs-devel

> From: Stephen Berman <stephen.berman@gmx.net>
> Cc: emacs-devel@gnu.org
> Date: Mon, 31 Jul 2017 11:20:10 +0200
> 
> (dired test-dir) reverts an existing Dired buffer, because of setting
> dired-auto-revert-buffer to t.  (When I step through the code, it is
> after reverting that point moves to the subdirectory line

By "the point moves" you mean what dired-restore-positions does?



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Question about test failure on Hydra
  2017-07-31  9:20       ` Stephen Berman
  2017-08-01  3:12         ` Eli Zaretskii
@ 2017-08-01  4:39         ` Tino Calancha
  2017-08-01  9:57           ` Stephen Berman
  1 sibling, 1 reply; 20+ messages in thread
From: Tino Calancha @ 2017-08-01  4:39 UTC (permalink / raw)
  To: Stephen Berman; +Cc: Eli Zaretskii, emacs-devel

Stephen Berman <stephen.berman@gmx.net> writes:

> On Mon, 31 Jul 2017 06:26:55 +0300 Eli Zaretskii <eliz@gnu.org> wrote:
>
> (dired test-dir) reverts an existing Dired buffer, because of setting
> dired-auto-revert-buffer to t.  (When I step through the code, it is
> after reverting that point moves to the subdirectory line, which the
> should make "sanity check" true (and does, both when I step through the
> code and just run the test in any way).  When I comment out the
> dired-auto-revert-buffer line, then point stays at point-min, which
> makes the sanity check fail (dired-file-name-at-point returns nil),
When i comment out i see the point in point-max, and the test fails
same as you but: (dired-file-name-at-point returns nil)

> though not in the way Hydra reports (it shows point being on the ".."
> entry).)
If somehow, we have in such Dired buffer the point at ".." _before_
call `dired-revert', then _after_ revert the point is preserved:

(let ((test-dir (make-temp-file "test-dir-" t))
        (dired-auto-revert-buffer t)
	buffers buf)
    (with-current-buffer (setq buf (find-file-noselect test-dir))
      (make-directory "test-subdir"))
    (with-current-buffer buf (forward-line -1))
    (push (dired test-dir) buffers)
    (with-current-buffer buf (dired-get-filename t t)))
=> ".."
;; Return "test-subdir" when comment out line:
(with-current-buffer buf (forward-line -1))

We can insert additional `should' calls in the failing tests and
wait until next hydra failoure.

(Following just add more should forms into `dired-test-bug27243-01'; we
might do the same in `dired-test-bug27243-02' and `dired-test-bug27243-03').

--8<-----------------------------cut here---------------start------------->8---
commit 5483d883f753b5a2e0b45bff2a659b8d27a017a9
Author: Tino Calancha <tino.calancha@gmail.com>
Date:   Tue Aug 1 13:18:28 2017 +0900

    * test/lisp/dired-tests.el (dired-test-bug27243-01): Add more should forms

diff --git a/test/lisp/dired-tests.el b/test/lisp/dired-tests.el
index d6fe839708..0ee4e13783 100644
--- a/test/lisp/dired-tests.el
+++ b/test/lisp/dired-tests.el
@@ -122,11 +122,18 @@
 
 (ert-deftest dired-test-bug27243-01 ()
   "Test for https://debbugs.gnu.org/cgi/bugreport.cgi?bug=27243#5 ."
-  (let ((test-dir (make-temp-file "test-dir-" t))
+  (let ((test-dir (file-name-as-directory (make-temp-file "test-dir-" t)))
         (dired-auto-revert-buffer t) buffers)
+    (should-not (dired-buffers-for-dir test-dir))
     (with-current-buffer (find-file-noselect test-dir)
       (make-directory "test-subdir"))
+    ;; Point must be at end-of-buffer.
+    (with-current-buffer (car (dired-buffers-for-dir test-dir))
+      (should (eobp)))
     (push (dired test-dir) buffers)
+    ;; Previous dired call shouldn't create a new buffer: must visit the one
+    ;; created by `find-file-noselect' above.
+    (should (eq 1 (length (dired-buffers-for-dir test-dir))))
     (unwind-protect
         (let ((buf (current-buffer))
               (pt1 (point))
@@ -135,11 +142,10 @@
           (write-region "Test" nil test-file nil 'silent nil 'excl)
           ;; Sanity check: point should now be on the subdirectory.
           (should (equal (dired-file-name-at-point)
-                         (concat (file-name-as-directory test-dir)
-                                 (file-name-as-directory "test-subdir"))))
+                         (concat test-dir (file-name-as-directory "test-subdir"))))
           (push (dired-find-file) buffers)
           (let ((pt2 (point)))          ; Point is on test-file.
-            (switch-to-buffer buf)
+            (pop-to-buffer-same-window buf)
             ;; Sanity check: point should now be back on the subdirectory.
             (should (eq (point) pt1))
             (push (dired-find-file) buffers)
--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-07-31
Repository revision: 3a8d0cc825635e07da2a90c4ac987b476fc9b05d



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: Question about test failure on Hydra
  2017-08-01  3:12         ` Eli Zaretskii
@ 2017-08-01  9:57           ` Stephen Berman
  2017-08-01 13:49             ` Eli Zaretskii
  0 siblings, 1 reply; 20+ messages in thread
From: Stephen Berman @ 2017-08-01  9:57 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

On Tue, 01 Aug 2017 06:12:49 +0300 Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Stephen Berman <stephen.berman@gmx.net>
>> Cc: emacs-devel@gnu.org
>> Date: Mon, 31 Jul 2017 11:20:10 +0200
>> 
>> (dired test-dir) reverts an existing Dired buffer, because of setting
>> dired-auto-revert-buffer to t.  (When I step through the code, it is
>> after reverting that point moves to the subdirectory line
>
> By "the point moves" you mean what dired-restore-positions does?

Yes.

Steve Berman



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Question about test failure on Hydra
  2017-08-01  4:39         ` Tino Calancha
@ 2017-08-01  9:57           ` Stephen Berman
  2017-08-01 10:17             ` Tino Calancha
  0 siblings, 1 reply; 20+ messages in thread
From: Stephen Berman @ 2017-08-01  9:57 UTC (permalink / raw)
  To: Tino Calancha; +Cc: Eli Zaretskii, emacs-devel

On Tue, 01 Aug 2017 13:39:50 +0900 Tino Calancha <tino.calancha@gmail.com> wrote:

> Stephen Berman <stephen.berman@gmx.net> writes:
>
>> On Mon, 31 Jul 2017 06:26:55 +0300 Eli Zaretskii <eliz@gnu.org> wrote:
>>
>> (dired test-dir) reverts an existing Dired buffer, because of setting
>> dired-auto-revert-buffer to t.  (When I step through the code, it is
>> after reverting that point moves to the subdirectory line, which the
>> should make "sanity check" true (and does, both when I step through the
>> code and just run the test in any way).  When I comment out the
>> dired-auto-revert-buffer line, then point stays at point-min, which
>> makes the sanity check fail (dired-file-name-at-point returns nil),
> When i comment out i see the point in point-max, and the test fails
> same as you but: (dired-file-name-at-point returns nil)

I'm seeing point at point-max now, too, but I did see it at point-min
yesterday and also when I first tried again after reading your mail,
though, strangely, I can't reproduce that now.  I was testing with two
frames and switched back and forth between them, maybe that affects
point.

>> though not in the way Hydra reports (it shows point being on the ".."
>> entry).)
> If somehow, we have in such Dired buffer the point at ".." _before_
> call `dired-revert', then _after_ revert the point is preserved:

Yes, but the question is, how does point get there in Hydra?

> We can insert additional `should' calls in the failing tests and
> wait until next hydra failoure.
>
> (Following just add more should forms into `dired-test-bug27243-01'; we
> might do the same in `dired-test-bug27243-02' and `dired-test-bug27243-03').

That's probably a good idea; can you add them?  (AFAIK
dired-test-bug27243-03 hasn't failed in Hydra, though I can't access the
logs right now to check more recent builds, but I guess it's fine to add
more sanity checks.  As for replacing switch-to-buffer by
pop-to-buffer-same-window, I used the former in an attempt to mimic
typing `C-x b', which the recipes in the bug reports use, but since it's
not being called interactively, it's probably not a valid attempt
anyway.)

Steve Berman



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Question about test failure on Hydra
  2017-08-01  9:57           ` Stephen Berman
@ 2017-08-01 10:17             ` Tino Calancha
  2017-08-01 10:47               ` Stephen Berman
  2017-08-05 13:07               ` Tino Calancha
  0 siblings, 2 replies; 20+ messages in thread
From: Tino Calancha @ 2017-08-01 10:17 UTC (permalink / raw)
  To: Stephen Berman; +Cc: Eli Zaretskii, emacs-devel, Tino Calancha



On Tue, 1 Aug 2017, Stephen Berman wrote:

> On Tue, 01 Aug 2017 13:39:50 +0900 Tino Calancha <tino.calancha@gmail.com> wrote:
>
>> Stephen Berman <stephen.berman@gmx.net> writes:
>>
>>> On Mon, 31 Jul 2017 06:26:55 +0300 Eli Zaretskii <eliz@gnu.org> wrote:
>>>
>>> (dired test-dir) reverts an existing Dired buffer, because of setting
>>> dired-auto-revert-buffer to t.  (When I step through the code, it is
>>> after reverting that point moves to the subdirectory line, which the
>>> should make "sanity check" true (and does, both when I step through the
>>> code and just run the test in any way).  When I comment out the
>>> dired-auto-revert-buffer line, then point stays at point-min, which
>>> makes the sanity check fail (dired-file-name-at-point returns nil),
>> When i comment out i see the point in point-max, and the test fails
>> same as you but: (dired-file-name-at-point returns nil)
>
> I'm seeing point at point-max now, too, but I did see it at point-min
> yesterday and also when I first tried again after reading your mail,
> though, strangely, I can't reproduce that now.  I was testing with two
> frames and switched back and forth between them, maybe that affects
> point.
I've made last 24 hours, two commits that might have impact on the 
issue at hands:
6ebef3daf24c847d6f16621489ae587e98c11ec0
192342a3a93a2e467ab589ae2d1ffd5e7acf1398

>>> though not in the way Hydra reports (it shows point being on the ".."
>>> entry).)
>> If somehow, we have in such Dired buffer the point at ".." _before_
>> call `dired-revert', then _after_ revert the point is preserved:
>
> Yes, but the question is, how does point get there in Hydra?
That's 1 million dollard question (but i cannot pay such amount to whom 
find the answer: just invite 1 icescream).
>
>> We can insert additional `should' calls in the failing tests and
>> wait until next hydra failoure.
>>
>> (Following just add more should forms into `dired-test-bug27243-01'; we
>> might do the same in `dired-test-bug27243-02' and `dired-test-bug27243-03').
>
> That's probably a good idea; can you add them?
I did, just on 'dired-test-bug27243-01' test.



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Question about test failure on Hydra
  2017-08-01 10:17             ` Tino Calancha
@ 2017-08-01 10:47               ` Stephen Berman
  2017-08-05 13:07               ` Tino Calancha
  1 sibling, 0 replies; 20+ messages in thread
From: Stephen Berman @ 2017-08-01 10:47 UTC (permalink / raw)
  To: Tino Calancha; +Cc: Eli Zaretskii, emacs-devel

On Tue, 1 Aug 2017 19:17:57 +0900 (JST) Tino Calancha <tino.calancha@gmail.com> wrote:

>>> We can insert additional `should' calls in the failing tests and
>>> wait until next hydra failoure.
>>>
>>> (Following just add more should forms into `dired-test-bug27243-01'; we
>>> might do the same in `dired-test-bug27243-02' and `dired-test-bug27243-03').
>>
>> That's probably a good idea; can you add them?
> I did, just on 'dired-test-bug27243-01' test.

Thanks.  Let's see what Hydra says.

Steve Berman



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Question about test failure on Hydra
  2017-08-01  9:57           ` Stephen Berman
@ 2017-08-01 13:49             ` Eli Zaretskii
  2017-08-01 15:23               ` Stephen Berman
  0 siblings, 1 reply; 20+ messages in thread
From: Eli Zaretskii @ 2017-08-01 13:49 UTC (permalink / raw)
  To: Stephen Berman; +Cc: emacs-devel

> From: Stephen Berman <stephen.berman@gmx.net>
> Cc: emacs-devel@gnu.org
> Date: Tue, 01 Aug 2017 11:57:14 +0200
> 
> On Tue, 01 Aug 2017 06:12:49 +0300 Eli Zaretskii <eliz@gnu.org> wrote:
> 
> >> From: Stephen Berman <stephen.berman@gmx.net>
> >> Cc: emacs-devel@gnu.org
> >> Date: Mon, 31 Jul 2017 11:20:10 +0200
> >> 
> >> (dired test-dir) reverts an existing Dired buffer, because of setting
> >> dired-auto-revert-buffer to t.  (When I step through the code, it is
> >> after reverting that point moves to the subdirectory line
> >
> > By "the point moves" you mean what dired-restore-positions does?
> 
> Yes.

Then I'd begin with logging the values saved in POSITIONS that
dired-restore-positions uses, perhaps the recorded positions are
sometimes wrong.  Then we could take it from there, either by looking
how the values are recorded or how they are restored.



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Question about test failure on Hydra
  2017-08-01 13:49             ` Eli Zaretskii
@ 2017-08-01 15:23               ` Stephen Berman
  2017-08-02 15:27                 ` Stephen Berman
  0 siblings, 1 reply; 20+ messages in thread
From: Stephen Berman @ 2017-08-01 15:23 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

On Tue, 01 Aug 2017 16:49:16 +0300 Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Stephen Berman <stephen.berman@gmx.net>
>> Cc: emacs-devel@gnu.org
>> Date: Tue, 01 Aug 2017 11:57:14 +0200
>> 
>> On Tue, 01 Aug 2017 06:12:49 +0300 Eli Zaretskii <eliz@gnu.org> wrote:
>> 
>> >> From: Stephen Berman <stephen.berman@gmx.net>
>> >> Cc: emacs-devel@gnu.org
>> >> Date: Mon, 31 Jul 2017 11:20:10 +0200
>> >> 
>> >> (dired test-dir) reverts an existing Dired buffer, because of setting
>> >> dired-auto-revert-buffer to t.  (When I step through the code, it is
>> >> after reverting that point moves to the subdirectory line
>> >
>> > By "the point moves" you mean what dired-restore-positions does?
>> 
>> Yes.
>
> Then I'd begin with logging the values saved in POSITIONS that
> dired-restore-positions uses, perhaps the recorded positions are
> sometimes wrong.  Then we could take it from there, either by looking
> how the values are recorded or how they are restored.

In commit c5305ff6 Tino Calancha added more assertions to track point,
so this may already be enough to locate the problem if the test fails in
Hydra again.  If it isn't, we can add still more.

Steve Berman



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Question about test failure on Hydra
  2017-08-01 15:23               ` Stephen Berman
@ 2017-08-02 15:27                 ` Stephen Berman
  0 siblings, 0 replies; 20+ messages in thread
From: Stephen Berman @ 2017-08-02 15:27 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Tino Calancha, emacs-devel

On Tue, 01 Aug 2017 17:23:36 +0200 Stephen Berman <stephen.berman@gmx.net> wrote:

> On Tue, 01 Aug 2017 16:49:16 +0300 Eli Zaretskii <eliz@gnu.org> wrote:
>
>>> From: Stephen Berman <stephen.berman@gmx.net>
>>> Cc: emacs-devel@gnu.org
>>> Date: Tue, 01 Aug 2017 11:57:14 +0200
>>> 
>>> On Tue, 01 Aug 2017 06:12:49 +0300 Eli Zaretskii <eliz@gnu.org> wrote:
>>> 
>>> >> From: Stephen Berman <stephen.berman@gmx.net>
>>> >> Cc: emacs-devel@gnu.org
>>> >> Date: Mon, 31 Jul 2017 11:20:10 +0200
>>> >> 
>>> >> (dired test-dir) reverts an existing Dired buffer, because of setting
>>> >> dired-auto-revert-buffer to t.  (When I step through the code, it is
>>> >> after reverting that point moves to the subdirectory line
>>> >
>>> > By "the point moves" you mean what dired-restore-positions does?
>>> 
>>> Yes.
>>
>> Then I'd begin with logging the values saved in POSITIONS that
>> dired-restore-positions uses, perhaps the recorded positions are
>> sometimes wrong.  Then we could take it from there, either by looking
>> how the values are recorded or how they are restored.
>
> In commit c5305ff6 Tino Calancha added more assertions to track point,
> so this may already be enough to locate the problem if the test fails in
> Hydra again.  If it isn't, we can add still more.

The test is still failing on Hydra in the same way (cf. the log of this
build: https://hydra.nixos.org/build/57876570).  I've added messages
that give the position values before and after the dired call, during
which dired-revert is called, which contains the only calls to
dired-save-positions and dired-restore-positions.  I also added the
messages in two more places before the assertion that fails on Hydra,
and for good measure once more after that assertion.  The resulting
messages (except the one before the dired call) should (and on my test
runs are) all be identical, but should differ on Hydra if point is
somehow moving.

Steve Berman



^ permalink raw reply	[flat|nested] 20+ messages in thread

* tagging ERT tests with bug numbers (was: Question about test failure on Hydra)
  2017-07-30 15:50 Question about test failure on Hydra Stephen Berman
  2017-07-30 15:59 ` Eli Zaretskii
@ 2017-08-03 19:41 ` Ted Zlatanov
  2017-08-04  7:22   ` tagging ERT tests with bug numbers Michael Albinus
  1 sibling, 1 reply; 20+ messages in thread
From: Ted Zlatanov @ 2017-08-03 19:41 UTC (permalink / raw)
  To: emacs-devel

On Sun, 30 Jul 2017 17:50:38 +0200 Stephen Berman <stephen.berman@gmx.net> wrote: 

SB> Part of the Dired tests for bug#27243 continues to fail intermittently
...
SB> (ert-deftest dired-test-bug27243-01 ()
SB>   "Test for https://debbugs.gnu.org/cgi/bugreport.cgi?bug=27243#5 ."

I think it would be terrific if we could tag ERT tests with the bug ID
and URL, so these could be shown automatically if the test failed.

Is there a way to do that? I know we can put properties onto symbols,
but maybe there's a way to tag ERT tests in particular? I don't see
anything in the docs besides predicates, which these tags are not.

Thanks
Ted




^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: tagging ERT tests with bug numbers
  2017-08-03 19:41 ` tagging ERT tests with bug numbers (was: Question about test failure on Hydra) Ted Zlatanov
@ 2017-08-04  7:22   ` Michael Albinus
  2017-08-04 13:28     ` Ted Zlatanov
  0 siblings, 1 reply; 20+ messages in thread
From: Michael Albinus @ 2017-08-04  7:22 UTC (permalink / raw)
  To: emacs-devel

Ted Zlatanov <tzz@lifelogs.com> writes:

> I think it would be terrific if we could tag ERT tests with the bug ID
> and URL, so these could be shown automatically if the test failed.
>
> Is there a way to do that? I know we can put properties onto symbols,
> but maybe there's a way to tag ERT tests in particular? I don't see
> anything in the docs besides predicates, which these tags are not.

ERT tests have the :tags list. We use it for example like

  :tags '(:expensive-test)

AFAIK, there are no restrictions on tags, so we could define our
own. Something like

  :tags '((:bug <nnn>))

And a helper function to show the bug, either via a url (derived from
the bug number), or a debbugs-gnu function.

> Thanks
> Ted

Best regards, Michael.



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: tagging ERT tests with bug numbers
  2017-08-04  7:22   ` tagging ERT tests with bug numbers Michael Albinus
@ 2017-08-04 13:28     ` Ted Zlatanov
  2017-08-04 14:50       ` Michael Albinus
  0 siblings, 1 reply; 20+ messages in thread
From: Ted Zlatanov @ 2017-08-04 13:28 UTC (permalink / raw)
  To: emacs-devel

On Fri, 04 Aug 2017 09:22:26 +0200 Michael Albinus <michael.albinus@gmx.de> wrote: 

MA> Ted Zlatanov <tzz@lifelogs.com> writes:
>> I think it would be terrific if we could tag ERT tests with the bug ID
>> and URL, so these could be shown automatically if the test failed.
>> 
>> Is there a way to do that? I know we can put properties onto symbols,
>> but maybe there's a way to tag ERT tests in particular? I don't see
>> anything in the docs besides predicates, which these tags are not.

MA> ERT tests have the :tags list. We use it for example like

MA>   :tags '(:expensive-test)

Huh. That's not in the ERT docs at all AFAIK. It's in the `ert-deftest'
docstring if you squint hard enough.

MA> AFAIK, there are no restrictions on tags, so we could define our
MA> own. Something like

MA>   :tags '((:bug <nnn>))

MA> And a helper function to show the bug, either via a url (derived from
MA> the bug number), or a debbugs-gnu function.

Yeah, that would be OK, although I personally find a plain plist more
readable. Either way, it would be useful. Do you want to officially
define the tag format and I can update the docs?

Ted




^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: tagging ERT tests with bug numbers
  2017-08-04 13:28     ` Ted Zlatanov
@ 2017-08-04 14:50       ` Michael Albinus
  0 siblings, 0 replies; 20+ messages in thread
From: Michael Albinus @ 2017-08-04 14:50 UTC (permalink / raw)
  To: emacs-devel

Ted Zlatanov <tzz@lifelogs.com> writes:

> MA> ERT tests have the :tags list. We use it for example like
>
> MA>   :tags '(:expensive-test)
>
> Huh. That's not in the ERT docs at all AFAIK. It's in the `ert-deftest'
> docstring if you squint hard enough.

That's why I wear glasses for 50+ years.

> MA> AFAIK, there are no restrictions on tags, so we could define our
> MA> own. Something like
>
> MA>   :tags '((:bug <nnn>))
>
> MA> And a helper function to show the bug, either via a url (derived from
> MA> the bug number), or a debbugs-gnu function.
>
> Yeah, that would be OK, although I personally find a plain plist more
> readable. Either way, it would be useful. Do you want to officially
> define the tag format and I can update the docs?

Pushed on my TODO. Don't expect me to do it immediately, I'm in hard
fight with file name handlers for vc and/or magit.

I'll wait for some few days, maybe other people comment about.

> Ted

Best regards, Michael.



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Question about test failure on Hydra
  2017-08-01 10:17             ` Tino Calancha
  2017-08-01 10:47               ` Stephen Berman
@ 2017-08-05 13:07               ` Tino Calancha
  2017-08-05 21:34                 ` Stephen Berman
  1 sibling, 1 reply; 20+ messages in thread
From: Tino Calancha @ 2017-08-05 13:07 UTC (permalink / raw)
  To: Stephen Berman; +Cc: Eli Zaretskii, emacs-devel

Tino Calancha <tino.calancha@gmail.com> writes:

> On Tue, 1 Aug 2017, Stephen Berman wrote:
>
>> On Tue, 01 Aug 2017 13:39:50 +0900 Tino Calancha <tino.calancha@gmail.com> wrote:
>>
>>> Stephen Berman <stephen.berman@gmx.net> writes:
>>>
>>>> On Mon, 31 Jul 2017 06:26:55 +0300 Eli Zaretskii <eliz@gnu.org> wrote:
>>>>
>>>> (dired test-dir) reverts an existing Dired buffer, because of setting
>>>> dired-auto-revert-buffer to t.  (When I step through the code, it is
>>>> after reverting that point moves to the subdirectory line, which the
>>>> should make "sanity check" true (and does, both when I step through the
>>>> code and just run the test in any way).  When I comment out the
>>>> dired-auto-revert-buffer line, then point stays at point-min, which
>>>> makes the sanity check fail (dired-file-name-at-point returns nil),
>>> When i comment out i see the point in point-max, and the test fails
>>> same as you but: (dired-file-name-at-point returns nil)
>>
>>>> though not in the way Hydra reports (it shows point being on the ".."
>>>> entry).)
>>> If somehow, we have in such Dired buffer the point at ".." _before_
>>> call `dired-revert', then _after_ revert the point is preserved:
>>
>> Yes, but the question is, how does point get there in Hydra?
I got the answer: because the Dired header line changed its size.
*) Before revert, it looks like:
"  total used in directory 8 available 37449484"
**) After revert, it changed to something like:
"  total used in directory 12 available 37449480"
;; 1 char longer: that mess up the saved position.
(See http://debbugs.gnu.org/27968).
> That's 1 million dollard question
I want my million bucks in a bag in the usual place; please, in small
bills.  No cops.



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Question about test failure on Hydra
  2017-08-05 13:07               ` Tino Calancha
@ 2017-08-05 21:34                 ` Stephen Berman
  0 siblings, 0 replies; 20+ messages in thread
From: Stephen Berman @ 2017-08-05 21:34 UTC (permalink / raw)
  To: Tino Calancha; +Cc: Eli Zaretskii, emacs-devel

On Sat, 05 Aug 2017 22:07:38 +0900 Tino Calancha <tino.calancha@gmail.com> wrote:

> Tino Calancha <tino.calancha@gmail.com> writes:
>
>> On Tue, 1 Aug 2017, Stephen Berman wrote:
>>
>>> On Tue, 01 Aug 2017 13:39:50 +0900 Tino Calancha <tino.calancha@gmail.com> wrote:
>>>
>>>> Stephen Berman <stephen.berman@gmx.net> writes:
>>>>
>>>>> On Mon, 31 Jul 2017 06:26:55 +0300 Eli Zaretskii <eliz@gnu.org> wrote:
>>>>>
>>>>> (dired test-dir) reverts an existing Dired buffer, because of setting
>>>>> dired-auto-revert-buffer to t.  (When I step through the code, it is
>>>>> after reverting that point moves to the subdirectory line, which the
>>>>> should make "sanity check" true (and does, both when I step through the
>>>>> code and just run the test in any way).  When I comment out the
>>>>> dired-auto-revert-buffer line, then point stays at point-min, which
>>>>> makes the sanity check fail (dired-file-name-at-point returns nil),
>>>> When i comment out i see the point in point-max, and the test fails
>>>> same as you but: (dired-file-name-at-point returns nil)
>>>
>>>>> though not in the way Hydra reports (it shows point being on the ".."
>>>>> entry).)
>>>> If somehow, we have in such Dired buffer the point at ".." _before_
>>>> call `dired-revert', then _after_ revert the point is preserved:
>>>
>>> Yes, but the question is, how does point get there in Hydra?
> I got the answer: because the Dired header line changed its size.
> *) Before revert, it looks like:
> "  total used in directory 8 available 37449484"
> **) After revert, it changed to something like:
> "  total used in directory 12 available 37449480"
> ;; 1 char longer: that mess up the saved position.
> (See http://debbugs.gnu.org/27968).

Very well spotted!  And your fix LGTM too.

>> That's 1 million dollard question
> I want my million bucks in a bag in the usual place; please, in small
> bills.  No cops.

I'm afraid I only have very small bags, so it will take about a million
deliveries.

Steve Berman



^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2017-08-05 21:34 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-30 15:50 Question about test failure on Hydra Stephen Berman
2017-07-30 15:59 ` Eli Zaretskii
2017-07-30 20:23   ` Stephen Berman
2017-07-31  3:26     ` Eli Zaretskii
2017-07-31  9:20       ` Stephen Berman
2017-08-01  3:12         ` Eli Zaretskii
2017-08-01  9:57           ` Stephen Berman
2017-08-01 13:49             ` Eli Zaretskii
2017-08-01 15:23               ` Stephen Berman
2017-08-02 15:27                 ` Stephen Berman
2017-08-01  4:39         ` Tino Calancha
2017-08-01  9:57           ` Stephen Berman
2017-08-01 10:17             ` Tino Calancha
2017-08-01 10:47               ` Stephen Berman
2017-08-05 13:07               ` Tino Calancha
2017-08-05 21:34                 ` Stephen Berman
2017-08-03 19:41 ` tagging ERT tests with bug numbers (was: Question about test failure on Hydra) Ted Zlatanov
2017-08-04  7:22   ` tagging ERT tests with bug numbers Michael Albinus
2017-08-04 13:28     ` Ted Zlatanov
2017-08-04 14:50       ` Michael Albinus

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).