[PATCH 0/3] Adjust test for changes in Emacs 27

[PATCH 0/3] Adjust test for changes in Emacs 27

[Jonas Bernoulli]

Without these changes many tests fail on upcoming Emacs 27.1.
The details are in the commit messages.

     Cheers,

Jonas Bernoulli (3):
  gitignore: Ignore generated sphinx.config
  test: Deal with Emacs 27 switching to lexical scope by default
  test: Explicitly state that we want to sign with sender

 sphinx.config    | 4 ++++
 test/test-lib.el | 9 +++++++++
 test/test-lib.sh | 3 ++-
 3 files changed, 15 insertions(+), 1 deletion(-)
 create mode 100644 sphinx.config

-- 
2.26.0

[PATCH 1/3] gitignore: Ignore generated sphinx.config

[Jonas Bernoulli]

---
 sphinx.config | 4 ++++
 1 file changed, 4 insertions(+)
 create mode 100644 sphinx.config

diff --git a/sphinx.config b/sphinx.config
new file mode 100644
index 00000000..207614b4
--- /dev/null
+++ b/sphinx.config
@@ -0,0 +1,4 @@
+# Generated by configure, run from doc/conf.py
+tags.add('WITH_EMACS')
+tags.add('WITH_PYTHON')
+rsti_dir = '/home/jonas/.emacs.d/lib/notmuch/emacs'
-- 
2.26.0

Re: [PATCH 1/3] gitignore: Ignore generated sphinx.config

[David Bremner]

Jonas Bernoulli <jonas@bernoul.li> writes:

> ---
>  sphinx.config | 4 ++++
>  1 file changed, 4 insertions(+)
>  create mode 100644 sphinx.config
>
> diff --git a/sphinx.config b/sphinx.config
> new file mode 100644
> index 00000000..207614b4
> --- /dev/null
> +++ b/sphinx.config
> @@ -0,0 +1,4 @@
> +# Generated by configure, run from doc/conf.py
> +tags.add('WITH_EMACS')
> +tags.add('WITH_PYTHON')
> +rsti_dir = '/home/jonas/.emacs.d/lib/notmuch/emacs'

I don't think the patch dos what you think it does?

cheers,

d

Re: [PATCH 1/3] gitignore: Ignore generated sphinx.config

[Jonas Bernoulli]

David Bremner <david@tethera.net> writes:
> I don't think the patch dos what you think it does?

Indeed.  I'll post the correct commit in v2 but will
wait for the rest of the review before posting that.

[PATCH 2/3] test: Deal with Emacs 27 switching to lexical scope by default

[Jonas Bernoulli]

Starting with Emacs 27 undeclared variables in evaluated interactive
code uses lexical scope.  This includes code passed with '--eval' as
we do in the Emacs tests, which also happen to assume dynamic scope.

This can affect variables defined by libraries that we use.  We let-
bind such variables to change the behavior of functions which we then
call with these bindings in effect.  If these libraries are not loaded
beforehand, then the bindings are lexical and fail to have the effect
we intended.

At this time only 'smtpmail' has to be loaded explicitly (for the
variables let-bound in emacs_deliver_message and emacs_fcc_message).

'message' doesn't have to be loaded explicitly, because loading
'notmuch' (in 'run_emacs') already takes care of that, indirectly.

Our own testing-only variables also have to be declared explicitly.
We should have done that anyway, but because of how and where these
variables are used it was very easy to overlook that (i.e. it isn't
something the byte-compiler ever looks at).  Not so in Emacs 27
anymore; here this oversight caused four tests to fail.
---
 test/test-lib.el | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/test/test-lib.el b/test/test-lib.el
index b47b388e..579a20d5 100644
--- a/test/test-lib.el
+++ b/test/test-lib.el
@@ -22,6 +22,12 @@
 
 (require 'cl-lib)
 
+;; Ensure that the dynamic variables that are defined by this library
+;; are defined by the time that we let-bind them.  This is needed
+;; because starting with Emacs 27 undeclared variables in evaluated
+;; interactive code (such as our tests) use lexical scope.
+(require 'smtpmail)
+
 ;; `read-file-name' by default uses `completing-read' function to read
 ;; user input.  It does not respect `standard-input' variable which we
 ;; use in tests to provide user input.  So replace it with a plain
@@ -113,6 +119,9 @@ (defun add-hook-counter (hook)
 (add-hook-counter 'notmuch-hello-mode-hook)
 (add-hook-counter 'notmuch-hello-refresh-hook)
 
+(defvar notmuch-hello-mode-hook-counter -100)
+(defvar notmuch-hello-refresh-hook-counter -100)
+
 (defadvice notmuch-search-process-filter (around pessimal activate disable)
   "Feed notmuch-search-process-filter one character at a time."
   (let ((string (ad-get-arg 1)))
-- 
2.26.0

[PATCH 3/3] test: Explicitly state that we want to sign with sender

[Jonas Bernoulli]

Since Emacs 27 'mml-secure-epg-sign' errors out if we don't opt-in to
signing as the sender using 'mml-secure-openpgp-sign-with-sender'.
---
 test/test-lib.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 8c331b88..90e26639 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -414,7 +414,8 @@ emacs_fcc_message ()
 	   (message-goto-body)
 	   (insert \"${body}\")
 	   $*
-	   (notmuch-mua-send-and-exit))" || return 1
+	   (let ((mml-secure-openpgp-sign-with-sender t))
+		 (notmuch-mua-send-and-exit)))" || return 1
     notmuch new $nmn_args >/dev/null
 }
 
-- 
2.26.0

Re: [PATCH 3/3] test: Explicitly state that we want to sign with sender

[David Bremner]

Jonas Bernoulli <jonas@bernoul.li> writes:

> Since Emacs 27 'mml-secure-epg-sign' errors out if we don't opt-in to
> signing as the sender using 'mml-secure-openpgp-sign-with-sender'.
> ---
>  test/test-lib.sh | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index 8c331b88..90e26639 100644
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -414,7 +414,8 @@ emacs_fcc_message ()
>  	   (message-goto-body)
>  	   (insert \"${body}\")
>  	   $*
> -	   (notmuch-mua-send-and-exit))" || return 1
> +	   (let ((mml-secure-openpgp-sign-with-sender t))
> +		 (notmuch-mua-send-and-exit)))" || return 1
>      notmuch new $nmn_args >/dev/null

It seems that notmuch-emacs users will have to set this variable, or
perform some other configuration to keep signing working when upgrading
to 27.1. Should notmuch be helping this transition somehow?

d

[PATCH 0/3] Adjust test for changes in Emacs 27

[Jonas Bernoulli]

The first commit didn't do what the commit message claimed, as pointed
out by David.  I have fixed that, and also a second issue that I have
missed because I still had some parts of 'mml-sec.el' commented out
when testing the fixes to the tests... it was a long day.

Jonas Bernoulli (3):
  gitignore: Ignore generated sphinx.config
  test: Deal with Emacs 27 switching to lexical scope by default
  test: Explicitly state that we want to sign with sender

 .gitignore       | 1 +
 test/test-lib.el | 9 +++++++++
 test/test-lib.sh | 4 +++-
 3 files changed, 13 insertions(+), 1 deletion(-)

-- 
2.26.0

[PATCH 1/3] gitignore: Ignore generated sphinx.config

[Jonas Bernoulli]

---
 .gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitignore b/.gitignore
index 1c8705ec..8f3ebec0 100644
--- a/.gitignore
+++ b/.gitignore
@@ -17,3 +17,4 @@ tags
 /.stamps
 *.stamp
 /bindings/python-cffi/build/
+/sphinx.config
-- 
2.26.0

Re: [PATCH 1/3] gitignore: Ignore generated sphinx.config

[Tomi Ollila]

On Sun, Jul 26 2020, Jonas Bernoulli wrote:

> ---
>  .gitignore | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/.gitignore b/.gitignore
> index 1c8705ec..8f3ebec0 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -17,3 +17,4 @@ tags
>  /.stamps
>  *.stamp
>  /bindings/python-cffi/build/
> +/sphinx.config

This could have been listed after sh.config...

Tomi

> -- 
> 2.26.0

Re: [PATCH 1/3] gitignore: Ignore generated sphinx.config

[Jonas Bernoulli]

Tomi Ollila <tomi.ollila@iki.fi> writes:

> On Sun, Jul 26 2020, Jonas Bernoulli wrote:
>
>> ---
>>  .gitignore | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/.gitignore b/.gitignore
>> index 1c8705ec..8f3ebec0 100644
>> --- a/.gitignore
>> +++ b/.gitignore
>> @@ -17,3 +17,4 @@ tags
>>  /.stamps
>>  *.stamp
>>  /bindings/python-cffi/build/
>> +/sphinx.config
>
> This could have been listed after sh.config...

I was unable to detect any order in the existing data so I placed it
at the end.  I'd be happy to place it somewhere else but I think the
existing data should just be sorted consistently.  If that happens
before or after this commit doesn't really matter.

     Jonas

[PATCH 2/3] test: Deal with Emacs 27 switching to lexical scope by default

[Jonas Bernoulli]

Starting with Emacs 27 undeclared variables in evaluated interactive
code uses lexical scope.  This includes code passed with '--eval' as
we do in the Emacs tests, which also happen to assume dynamic scope.

This can affect variables defined by libraries that we use.  We let-
bind such variables to change the behavior of functions which we then
call with these bindings in effect.  If these libraries are not loaded
beforehand, then the bindings are lexical and fail to have the effect
we intended.

At this time only 'smtpmail' has to be loaded explicitly (for the
variables let-bound in emacs_deliver_message and emacs_fcc_message).

'message' doesn't have to be loaded explicitly, because loading
'notmuch' (in 'run_emacs') already takes care of that, indirectly.

Our own testing-only variables also have to be declared explicitly.
We should have done that anyway, but because of how and where these
variables are used it was very easy to overlook that (i.e. it isn't
something the byte-compiler ever looks at).  Not so in Emacs 27
anymore; here this oversight caused four tests to fail.
---
 test/test-lib.el | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/test/test-lib.el b/test/test-lib.el
index b47b388e..579a20d5 100644
--- a/test/test-lib.el
+++ b/test/test-lib.el
@@ -22,6 +22,12 @@
 
 (require 'cl-lib)
 
+;; Ensure that the dynamic variables that are defined by this library
+;; are defined by the time that we let-bind them.  This is needed
+;; because starting with Emacs 27 undeclared variables in evaluated
+;; interactive code (such as our tests) use lexical scope.
+(require 'smtpmail)
+
 ;; `read-file-name' by default uses `completing-read' function to read
 ;; user input.  It does not respect `standard-input' variable which we
 ;; use in tests to provide user input.  So replace it with a plain
@@ -113,6 +119,9 @@ (defun add-hook-counter (hook)
 (add-hook-counter 'notmuch-hello-mode-hook)
 (add-hook-counter 'notmuch-hello-refresh-hook)
 
+(defvar notmuch-hello-mode-hook-counter -100)
+(defvar notmuch-hello-refresh-hook-counter -100)
+
 (defadvice notmuch-search-process-filter (around pessimal activate disable)
   "Feed notmuch-search-process-filter one character at a time."
   (let ((string (ad-get-arg 1)))
-- 
2.26.0

Re: [PATCH 2/3] test: Deal with Emacs 27 switching to lexical scope by default

[David Bremner]

Jonas Bernoulli <jonas@bernoul.li> writes:

>  
> +(defvar notmuch-hello-mode-hook-counter -100)
> +(defvar notmuch-hello-refresh-hook-counter -100)
> +
>  (defadvice notmuch-search-process-filter (around pessimal activate disable)
>    "Feed notmuch-search-process-filter one character at a time."
>    (let ((string (ad-get-arg 1)))

This is overall fine, but if you were going to respin the series anyway
for something else, then a comment on the unusual initialization values
would not go amiss. Out of curiousity, why not nil?

d

[PATCH 3/3] test: Explicitly state that we want to sign with sender

[Jonas Bernoulli]

Since Emacs 27 'mml-secure-epg-sign' errors out if we don't opt-in to
signing as the sender using 'mml-secure-openpgp-sign-with-sender'.
---
 test/test-lib.sh | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 8c331b88..31c858d1 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -414,7 +414,9 @@ emacs_fcc_message ()
 	   (message-goto-body)
 	   (insert \"${body}\")
 	   $*
-	   (notmuch-mua-send-and-exit))" || return 1
+	   (let ((mml-secure-smime-sign-with-sender t)
+		 (mml-secure-openpgp-sign-with-sender t))
+	     (notmuch-mua-send-and-exit)))" || return 1
     notmuch new $nmn_args >/dev/null
 }
 
-- 
2.26.0

[PATCH v3 0/4] Adjust test for changes in Emacs 27

[Jonas Bernoulli]

This reroll explains why the variable's default value is -100.
It also adds an additional commit, which removes a misguided
abstraction that is related to those very same variables.

Jonas Bernoulli (4):
  gitignore: Ignore generated sphinx.config
  test: Deal with Emacs 27 switching to lexical scope by default
  test: Remove misguided emacs testing utilities
  test: Explicitly state that we want to sign with sender

 .gitignore       |  1 +
 test/test-lib.el | 27 ++++++++++++++-------------
 test/test-lib.sh |  4 +++-
 3 files changed, 18 insertions(+), 14 deletions(-)

-- 
2.26.0

[PATCH v3 1/4] gitignore: Ignore generated sphinx.config

[Jonas Bernoulli]

---
 .gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitignore b/.gitignore
index 1c8705ec..8f3ebec0 100644
--- a/.gitignore
+++ b/.gitignore
@@ -17,3 +17,4 @@ tags
 /.stamps
 *.stamp
 /bindings/python-cffi/build/
+/sphinx.config
-- 
2.26.0

[PATCH v3 2/4] test: Deal with Emacs 27 switching to lexical scope by default

[Jonas Bernoulli]

Starting with Emacs 27 undeclared variables in evaluated interactive
code uses lexical scope.  This includes code passed with '--eval' as
we do in the Emacs tests, which also happen to assume dynamic scope.

- This can affect variables defined by libraries that we use.  We
  let-bind such variables to change the behavior of functions which we
  then call with these bindings in effect.  If these libraries are not
  loaded beforehand, then the bindings are lexical and fail to have
  the effect we intended.

  At this time only 'smtpmail' has to be loaded explicitly (for the
  variables let-bound in emacs_deliver_message and emacs_fcc_message).

  'message' doesn't have to be loaded explicitly, because loading
  'notmuch' (in 'run_emacs') already takes care of that, indirectly.

- Our own testing-only variables also have to be declared explicitly.
  We should have done that anyway, but because of how and where these
  variables are used it was very easy to overlook that (i.e. it isn't
  something the byte-compiler ever looks at).  Not so in Emacs 27
  anymore; here this oversight caused four tests to fail.

  The numeric values of these variables get incremented by functions
  that we add to hooks that are run by many tests, not just the tests
  where we actually inspect the value and therefore take care to let-
  bind the values to 0 before we begin.  The global values therefore
  have to be numeric values as well.  I have chosen -100 instead of 0
  as the default in case someone writes a test that inspects the value
  but forgets to let-bind the value.  I hope that the unusual negative
  value that one is going to see in such a case will help debugging
  the issue.
---
 test/test-lib.el | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/test/test-lib.el b/test/test-lib.el
index b47b388e..15271b02 100644
--- a/test/test-lib.el
+++ b/test/test-lib.el
@@ -22,6 +22,12 @@
 
 (require 'cl-lib)
 
+;; Ensure that the dynamic variables that are defined by this library
+;; are defined by the time that we let-bind them.  This is needed
+;; because starting with Emacs 27 undeclared variables in evaluated
+;; interactive code (such as our tests) use lexical scope.
+(require 'smtpmail)
+
 ;; `read-file-name' by default uses `completing-read' function to read
 ;; user input.  It does not respect `standard-input' variable which we
 ;; use in tests to provide user input.  So replace it with a plain
@@ -113,6 +119,12 @@ (defun add-hook-counter (hook)
 (add-hook-counter 'notmuch-hello-mode-hook)
 (add-hook-counter 'notmuch-hello-refresh-hook)
 
+(defvar notmuch-hello-mode-hook-counter -100
+  "Tests that care about this counter must let-bind it to 0.")
+
+(defvar notmuch-hello-refresh-hook-counter -100
+  "Tests that care about this counter must let-bind it to 0.")
+
 (defadvice notmuch-search-process-filter (around pessimal activate disable)
   "Feed notmuch-search-process-filter one character at a time."
   (let ((string (ad-get-arg 1)))
-- 
2.26.0

[PATCH v3 3/4] test: Remove misguided emacs testing utilities

[Jonas Bernoulli]

The goal of this abstraction was to save space.  But that failed as
the result actually was that four trivial lines got replace with 15
fairly complicated lines.  The opposite of what it was supposed to
do.

Also it made it harder to come up with the fix in the previous commit;
simply grepping for the relevant symbols did not work because they get
constructed at run-time instead of appearing in the source file.
---
 test/test-lib.el | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/test/test-lib.el b/test/test-lib.el
index 15271b02..aae9e833 100644
--- a/test/test-lib.el
+++ b/test/test-lib.el
@@ -104,26 +104,15 @@ (defun orphan-watchdog (pid)
   "Initiate orphan watchdog check."
   (run-at-time 60 60 'orphan-watchdog-check pid))
 
-(defun hook-counter (hook)
-  "Count how many times a hook is called.  Increments
-`hook'-counter variable value if it is bound, otherwise does
-nothing."
-  (let ((counter (intern (concat (symbol-name hook) "-counter"))))
-    (if (boundp counter)
-	(set counter (1+ (symbol-value counter))))))
-
-(defun add-hook-counter (hook)
-  "Add hook to count how many times `hook' is called."
-  (add-hook hook (apply-partially 'hook-counter hook)))
-
-(add-hook-counter 'notmuch-hello-mode-hook)
-(add-hook-counter 'notmuch-hello-refresh-hook)
-
 (defvar notmuch-hello-mode-hook-counter -100
   "Tests that care about this counter must let-bind it to 0.")
+(add-hook 'notmuch-hello-mode-hook
+	  (lambda () (cl-incf notmuch-hello-mode-hook-counter)))
 
 (defvar notmuch-hello-refresh-hook-counter -100
   "Tests that care about this counter must let-bind it to 0.")
+(add-hook 'notmuch-hello-refresh-hook
+	  (lambda () (cl-incf notmuch-hello-refresh-hook-counter)))
 
 (defadvice notmuch-search-process-filter (around pessimal activate disable)
   "Feed notmuch-search-process-filter one character at a time."
-- 
2.26.0

[PATCH v3 4/4] test: Explicitly state that we want to sign with sender

[Jonas Bernoulli]

Since Emacs 27 'mml-secure-epg-sign' errors out if we don't opt-in to
signing as the sender using 'mml-secure-openpgp-sign-with-sender'.
---
 test/test-lib.sh | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 8c331b88..31c858d1 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -414,7 +414,9 @@ emacs_fcc_message ()
 	   (message-goto-body)
 	   (insert \"${body}\")
 	   $*
-	   (notmuch-mua-send-and-exit))" || return 1
+	   (let ((mml-secure-smime-sign-with-sender t)
+		 (mml-secure-openpgp-sign-with-sender t))
+	     (notmuch-mua-send-and-exit)))" || return 1
     notmuch new $nmn_args >/dev/null
 }
 
-- 
2.26.0

Re: [PATCH v3 0/4] Adjust test for changes in Emacs 27

[David Bremner]

Jonas Bernoulli <jonas@bernoul.li> writes:

> This reroll explains why the variable's default value is -100.
> It also adds an additional commit, which removes a misguided
> abstraction that is related to those very same variables.
>
> Jonas Bernoulli (4):
>   gitignore: Ignore generated sphinx.config
>   test: Deal with Emacs 27 switching to lexical scope by default
>   test: Remove misguided emacs testing utilities
>   test: Explicitly state that we want to sign with sender

Applied v3 to master.

Sorting .gitignores left for a future cleanup.

d

Re: [PATCH v3 0/4] Adjust test for changes in Emacs 27

[Jonas Bernoulli]

David Bremner <david@tethera.net> writes:
> Applied v3 to master.

Thanks.
Cheers!
  Jonas