Re: no gnus registry new dependency on ert?

Re: no gnus registry new dependency on ert?

[Ted Zlatanov]

On Wed, 06 Apr 2011 12:28:17 +0800 Eric Abrahamsen <eric@ericabrahamsen.net> wrote: 

EA> The recent registry rewrite has introduced a dependency on the ERT
EA> testing library -- that library is part of emacs 24 but not emacs 23.

Hmm, I'm actually not sure how to provide the ERT tests only if ERT is
available.  Is there an example?  It seems like:

(require 'ert nil t)
...
(when (featurep 'ert)
  (ert-deftest gnus-registry-usage-test ()
  ...
)

should work but I want to be sure it will work properly.

EA> That made for an awkward code update, is there any way to warn people of
EA> what's happening, or even include a local installation of ERT for those
EA> who don't have it already?

All the registry code will run fine without the ERT tests and I don't
think we need to ship ERT with Gnus.  So I'd rather make it optional.

Ted

Re: no gnus registry new dependency on ert?

[Ted Zlatanov]

On Wed, 06 Apr 2011 12:10:26 +0200 David Engster <deng@randomsample.de> wrote: 

DE> Either that, or just do

DE> (eval-when-compile
DE>   (when (null (require 'ert nil t))
DE>     (defmacro* ert-deftest (name () &body docstring-keys-and-body))))

DE> to silence the byte-compiler. Your solution is less hacky, but then you
DE> have to always remember to wrap ert-deftest in those featurep clauses.

Oh, I like your solution better since I don't have to reindent ;)

I made the change in *registry.el, thanks.

Ted

Re: no gnus registry new dependency on ert?

[Ted Zlatanov]

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

On Wed, 06 Apr 2011 05:54:46 -0500 Ted Zlatanov <tzz@lifelogs.com> wrote: 

TZ> On Wed, 06 Apr 2011 12:10:26 +0200 David Engster <deng@randomsample.de> wrote: 
DE> Either that, or just do

DE> (eval-when-compile
DE> (when (null (require 'ert nil t))
DE> (defmacro* ert-deftest (name () &body docstring-keys-and-body))))

DE> to silence the byte-compiler. Your solution is less hacky, but then you
DE> have to always remember to wrap ert-deftest in those featurep clauses.

TZ> Oh, I like your solution better since I don't have to reindent ;)

TZ> I made the change in *registry.el, thanks.

Interesting.  When I byte-compile in Gnus (just "make" in the Lisp
directory), I get the attached backtrace.  Sorry for the nasty backtrace
but I can't load the code AND exhibit the problem.

If I load gnus-registry.el and do `M-x eval-buffer' then it works
properly.  So something is off with the byte-compilation, at least the
compilation that Gnus does.

Ted


[-- Attachment #2: ert-backtrace.txt --]
[-- Type: text/plain, Size: 9934 bytes --]

Debugger entered--Lisp error: (void-function make-ert-test)
  make-ert-test(:name registry-instantiation-test :body #[nil "\304\305C\x18\x19\306\x1a\307^[\310\216\311	\b\"\211\x12)\204\x19\0\312\v!\210)\n+\207" [args-65 fn-64 value-66 form-description-68 registry-db "Testing" ert-form-evaluation-aborted-67 nil ((byte-code "\305C\306\b	BD\244\n\307=?\205\x12\0\310\nD\244\311\312N\211^[\205!\0\313\314\v	\"D)\244\x14\315\f!\207" [fn-64 args-65 value-66 -explainer- form-description-68 (should (registry-db "Testing")) :form ert-form-evaluation-aborted-67 :value registry-db ert-explainer :explanation apply ert--signal-should-execution] 6)) apply ert-fail] 3])
  byte-code("\301\302\303\304#\210\301\305\306\307#\210\301\310\311\312#\210\301\313\314\315#\210\301\316\317\320#\210\321\322\323\324\322\325\326$\"\210\327\bB\x10\321\330\323\324\330\325\331$\"\210\332\bB\x10\301\207" [current-load-list eieio-defmethod registry-search (((db registry-db) &rest spec)) #[(db &rest spec) "\b\205 \0\306	\307\"\306	\310\"\306	\311\"\x1a^[\x1c\312\x1d\313\314\315\b\316\"\"\210\r\237,\207" [db spec regex member all --cl-var-- plist-get :all :member :regex nil maphash #[(k v) "\b\204\x1c\0	\203\x10\0\306\307\n	#\204\x1c\0\v\205!\0\306\310\n\v#\205!\0\f\rB\211\x15\207" [all member v regex k --cl-var-- registry--match :member :regex] 4] eieio-oref :data] 5 "Search for SPEC across the registry-db THIS.\nFor example calling with :member '(a 1 2) will match entry '((a 3 1)).\nCalling with :all t (any non-nil value) will match all.\nCalling with :regex '(a \"h.llo\") will match entry '((a \"hullo\" \"bye\").\nThe test order is to check :all first, then :member, then :regex."] registry-delete (((db registry-db) keys assert &rest spec)) #[(db keys assert &rest spec) "\306\b\307\"\x19\n\206\x0e\0\310\311\b\v#\x1a\306\b\312\"\x1c\n\313\x1d\211\x1e\x14\203\246\0\x0e\x14@\x15\314\r	\"\x1e\x15\x0e\x16\2036\0\x0e\x15\2046\0\315\316\r\"\210\f\313\x1e\x17\211\x1e\x18\203\226\0\x0e\x18@\x16\x17\317\b\x0e\x17\"\203\215\0\x0e\x17\x0e\x15\236\243\313\x1e\x19\211\x1e\x1a\203\214\0\x0e\x1a@\x16\x19\320\b\x0e\x17\x0e\x19#\x1e^[\r\x0e^[\235\203\202\0\320\b\x0e\x17\x0e\x19\321\r\x0e^[\"\206\200\0\322$\210)\x0e\x1aA\211\x16\x1a\204\\\0*\x0e\x18A\211\x16\x18\204@\0*\323\r	\"\210)\x0e\x14A\211\x16\x14\204\x1d\0*\n+\207" [db data keys spec tracked key eieio-oref :data apply registry-search :tracked nil gethash error "Key %s does not exists in database" registry-lookup-secondary registry-lookup-secondary-value delete t remhash --cl-dolist-temp-- entry assert tr --cl-dolist-temp-- val --cl-dolist-temp-- value-keys] 8 "Delete KEYS from the registry-db THIS.\nIf KEYS is nil, use SPEC to do a search.\nUpdates the secondary ('tracked') indices as well.\nWith assert non-nil, errors out if the key does not exist already."] registry-insert (((db registry-db) key entry)) #[(db key entry) "\306\b\307	\310\"\"\203\x0e\0\311\312!\210\313	!\307	\314\"W\204\x1d\0\311\315!\210\316\b\n\307	\310\"#\210\307	\317\"\320^[\211\x1c\203w\0\f@\211\x13\n\236\243\320\x1d\211\x1e\x12\203o\0\x0e\x12@\x15\321	\v\r#\x1e\x13\b\x0e\x13\235\203W\0\x0e\x13\202[\0\b\x0e\x13B\x16\x13\321	\v\r\x0e\x13$\210)\x0e\x12A\211\x16\x12\204@\0*\fA\211\x14\2041\0*\n\207" [key db entry tr --cl-dolist-temp-- val gethash eieio-oref :data error "Key already exists in database" registry-size :max-hard "max-hard size limit reached" puthash :tracked nil registry-lookup-secondary-value --cl-dolist-temp-- value-keys] 7 "Insert ENTRY under KEY into the registry-db THIS.\nUpdates the secondary ('tracked') indices as well.\nErrors out if the key exists already."] registry-size (((db registry-db))) #[(db) "\301\302\b\303\"!\207" [db hash-table-count eieio-oref :data] 4 "Returns the size of the registry-db object THIS.\nThis is the key count of the :data slot."] registry-prune (((db registry-db))) #[(db) "\306\b\307\"\x19\310\x1a\306\b\311\"^[\306\b\312\"\x1c\313\b!\x1d\314\x1e\x10\315\316\v\"\210\x0e\x10\237)\211\x1e\x11G\x1e\x12\r\fV\x1e\x13\x0e\x12\r\fZV\203G\0\x0e\x11\203G\0\x0e\x12S\x16\x12\x0e\x11A\x16\x11\202,\0\317\b\x0e\x11\314#.\b\207" [db precious precious-p data limit size eieio-oref :precious #[(entry-key) "\b@	>A\207" [entry-key precious] 2] :data :max-soft registry-size nil maphash #[(k v) "\b	\306\x1a^[\x1c\307\310\f\v\n$+?\205\x18\0\r\x0e	B\211\x16	\207" [precious-p v cl-rest cl-seq cl-pred k nil apply some --cl-var--] 5] registry-delete --cl-var-- candidates candidates-count prune-needed] 5 "Prunes the registry-db object THIS.\nRemoves only entries without the :precious keys."] ert-set-test registry-instantiation-test make-ert-test :name :body #[nil "\304\305C\x18\x19\306\x1a\307^[\310\216\311	\b\"\211\x12)\204\x19\0\312\v!\210)\n+\207" [args-65 fn-64 value-66 form-description-68 registry-db "Testing" ert-form-evaluation-aborted-67 nil ((byte-code "\305C\306\b	BD\244\n\307=?\205\x12\0\310\nD\244\311\312N\211^[\205!\0\313\314\v	\"D)\244\x14\315\f!\207" [fn-64 args-65 value-66 -explainer- form-description-68 (should (registry-db "Testing")) :form ert-form-evaluation-aborted-67 :value registry-db ert-explainer :explanation apply ert--signal-should-execution] 6)) apply ert-fail] 3] (ert-deftest . registry-instantiation-test) registry-match-test #[nil "\306\x18\307\310!\210\311\312\b\313E\x19\x1a\314^[\315\x1c\316\216\317\n	\"\211\x13)\204!\0\320\f!\210)\v\210+\311\312\b\321E\x1d\x1e,\322\x1e-\315\x1e.\323\216\317\x0e,\r\"\211\x16-)\204F\0\320\x0e.!\210)\x0e-\210+\311\312\b\324E\x1e/\x1e0\325\x1e1\315\x1e2\326\216\317\x0e0\x0e/\"\211\x161)\203n\0\320\x0e2!\210)\x0e1\210+\311\312\b\327E\x1e3\x1e4\330\x1e5\315\x1e6\331\216\317\x0e4\x0e3\"\211\x165)\203\226\0\320\x0e6!\210)\x0e5\210+\307\332!\210\311\333\b\334E\x1e7\x1e8\335\x1e9\315\x1e:\336\216\317\x0e8\x0e7\"\211\x169)\204\302\0\320\x0e:!\210)\x0e9\210+\311\333\b\337E\x1e;\x1e<\340\x1e=\315\x1e>\341\216\317\x0e<\x0e;\"\211\x16=)\204\352\0\320\x0e>!\210)\x0e=\210+\311\333\b\342E\x1e?\x1e@\343\x1eA\315\x1eB\344\216\317\x0e@\x0e?\"\211\x16A)\203\x12\x01\320\x0eB!\210)\x0eA\210+\311\333\b\345E\x1eC\x1eD\346\x1eE\315\x1eF\347\216\317\x0eD\x0eC\"\211\x16E)\203:\x01\320\x0eF!\210)\x0eE\210+\311\333\b\350E\x1eG\x1eH\351\x1eI\315\x1eJ\352\216\317\x0eH\x0eG\"\211\x16I)\203b\x01\320\x0eJ!\210)\x0eI\210,\307\353!\207" [entry args-70 fn-69 value-71 form-description-73 args-75 ((hello "goodbye" "bye") (blank)) message "Testing :regex matching" registry--match :regex ((hello "nye" "bye")) ert-form-evaluation-aborted-72 nil ((byte-code "\305C\306\b	BD\244\n\307=?\205\x12\0\310\nD\244\311\312N\211^[\205!\0\313\314\v	\"D)\244\x14\315\f!\207" [fn-69 args-70 value-71 -explainer- form-description-73 (should (registry--match :regex entry ...)) :form ert-form-evaluation-aborted-72 :value registry--match ert-explainer :explanation apply ert--signal-should-execution] 6)) apply ert-fail ((hello "good")) ert-form-evaluation-aborted-77 ((byte-code "\305C\306\b	BD\244\n\307=?\205\x12\0\310\nD\244\311\312N\211^[\205!\0\313\314\v	\"D)\244\x14\315\f!\207" [fn-74 args-75 value-76 -explainer- form-description-78 (should (registry--match :regex entry ...)) :form ert-form-evaluation-aborted-77 :value registry--match ert-explainer :explanation apply ert--signal-should-execution] 6)) ((hello "nye")) ert-form-evaluation-aborted-82 ((byte-code "\305C\306\b	BD\244\n\307=?\205\x12\0\310\nD\244\311\312N\211^[\205!\0\313\314\v	\"D)\244\x14\315\f!\207" [fn-79 args-80 value-81 -explainer- form-description-83 (should-not (registry--match :regex entry ...)) :form ert-form-evaluation-aborted-82 :value registry--match ert-explainer :explanation apply ert--signal-should-execution] 6)) ((hello)) ert-form-evaluation-aborted-87 ((byte-code "\305C\306\b	BD\244\n\307=?\205\x12\0\310\nD\244\311\312N\211^[\205!\0\313\314\v	\"D)\244\x14\315\f!\207" [fn-84 args-85 value-86 -explainer- form-description-88 (should-not (registry--match :regex entry ...)) :form ert-form-evaluation-aborted-87 :value registry--match ert-explainer :explanation apply ert--signal-should-execution] 6)) "Testing :member matching" :member ((hello "bye")) ert-form-evaluation-aborted-92 ((byte-code "\305C\306\b	BD\244\n\307=?\205\x12\0\310\nD\244\311\312N\211^[\205!\0\313\314\v	\"D)\244\x14\315\f!\207" [fn-89 args-90 value-91 -explainer- form-description-93 (should (registry--match :member entry ...)) :form ert-form-evaluation-aborted-92 :value registry--match ert-explainer :explanation apply ert--signal-should-execution] 6)) ((hello "goodbye")) ert-form-evaluation-aborted-97 ((byte-code "\305C\306\b	BD\244\n\307=?\205\x12\0\310\nD\244\311\312N\211^[\205!\0\313\314\v	\"D)\244\x14\315\f!\207" [fn-94 args-95 value-96 -explainer- form-description-98 (should (registry--match :member entry ...)) :form ert-form-evaluation-aborted-97 :value registry--match ert-explainer :explanation apply ert--signal-should-execution] 6)) ((hello "good")) ert-form-evaluation-aborted-102 ((byte-code "\305C\306\b	BD\244\n\307=?\205\x12\0\310\nD\244\311\312N\211^[\205!\0\313\314\v	\"D)\244\x14\315\f!\207" [fn-99 args-100 value-101 -explainer- form-description-103 (should-not (registry--match :member entry ...)) :form ert-form-evaluation-aborted-102 :value registry--match ert-explainer :explanation apply ert--signal-should-execution] 6)) ((hello "nye")) ert-form-evaluation-aborted-107 ((byte-code "\305C\306\b	BD\244\n\307=?\205\x12\0\310\nD\244\311\312N\211^[\205!\0\313\314\v	\"D)\244\x14\315\f!\207" [fn-104 args-105 value-106 -explainer- form-description-108 (should-not (registry--match :member entry ...)) :form ert-form-evaluation-aborted-107 :value registry--match ert-explainer :explanation apply ert--signal-should-execution] 6)) ((hello)) ert-form-evaluation-aborted-112 ((byte-code "\305C\306\b	BD\244\n\307=?\205\x12\0\310\nD\244\311\312N\211^[\205!\0\313\314\v	\"D)\244\x14\315\f!\207" [fn-109 args-110 value-111 -explainer- form-description-113 (should-not (registry--match :member entry ...)) :form ert-form-evaluation-aborted-112 :value registry--match ert-explainer :explanation apply ert--signal-should-execution] 6)) "Done with matching testing." fn-74 value-76 form-description-78 args-80 fn-79 value-81 ...] 4] (ert-deftest . registry-match-test)] 7)
  require(registry)
  byte-code("\300\301!\210\300\302!\210\300\303!\210\300\304!\210\300\305!\210\300\306!\210\300\307!\210\300\310!\207" [require gnus gnus-int gnus-sum gnus-art gnus-util nnmail easymenu registry] 2)
  (gnus-registry-initialize)
  eval-buffer(#<buffer  *load*> nil "/home/tzz/.gnus.el" nil t)  ; Reading at buffer position 1072
  load-with-code-conversion("/home/tzz/.gnus.el" "/home/tzz/.gnus.el" nil t)
  load("~/.gnus" nil t)
  gnus-read-init-file()
  gnus-1(nil nil nil)
  gnus(nil)
  call-interactively(gnus t nil)
  execute-extended-command(nil)
  call-interactively(execute-extended-command nil nil)

Re: no gnus registry new dependency on ert?

[David Engster]

Ted Zlatanov writes:
> On Wed, 06 Apr 2011 05:54:46 -0500 Ted Zlatanov <tzz@lifelogs.com> wrote: 
>
> TZ> On Wed, 06 Apr 2011 12:10:26 +0200 David Engster <deng@randomsample.de> wrote: 
> DE> Either that, or just do
>
> DE> (eval-when-compile
> DE> (when (null (require 'ert nil t))
> DE> (defmacro* ert-deftest (name () &body docstring-keys-and-body))))
>
> DE> to silence the byte-compiler. Your solution is less hacky, but then you
> DE> have to always remember to wrap ert-deftest in those featurep clauses.
>
> TZ> Oh, I like your solution better since I don't have to reindent ;)
>
> TZ> I made the change in *registry.el, thanks.
>
> Interesting.  When I byte-compile in Gnus (just "make" in the Lisp
> directory), I get the attached backtrace.  Sorry for the nasty backtrace
> but I can't load the code AND exhibit the problem.
>
> If I load gnus-registry.el and do `M-x eval-buffer' then it works
> properly.  So something is off with the byte-compilation, at least the
> compilation that Gnus does.

It seems like putting (require 'ert) into an eval-when-compile is not
sufficient. You have to require it unconditionally.

So maybe your first idea was The Right Thing To Do, meaning to use

(require 'ert nil t)

and then test for feature 'ert. Alternatively, you could use the current
hack and put this require in there, too. But this is getting
increasingly ugly...

-David

Re: no gnus registry new dependency on ert?

[Ted Zlatanov]

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

On Wed, 06 Apr 2011 15:28:07 +0200 David Engster <deng@randomsample.de> wrote: 

DE> It seems like putting (require 'ert) into an eval-when-compile is not
DE> sufficient. You have to require it unconditionally.

DE> So maybe your first idea was The Right Thing To Do, meaning to use

DE> (require 'ert nil t)

DE> and then test for feature 'ert. Alternatively, you could use the current
DE> hack and put this require in there, too. But this is getting
DE> increasingly ugly...

See the attached patch, which just adds (require 'ert nil t) *in
addition* to the defmacro hack.  It seems to DTRT for me, let me know
what you think.

Ted


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: registry-ert-loading.patch --]
[-- Type: text/x-diff, Size: 714 bytes --]

diff --git a/lisp/gnus-registry.el b/lisp/gnus-registry.el
index 511012d..6c660b1 100644
--- a/lisp/gnus-registry.el
+++ b/lisp/gnus-registry.el
@@ -61,6 +61,7 @@
   (when (null (require 'ert nil t))
     (defmacro* ert-deftest (name () &body docstring-keys-and-body))))
 
+(require 'ert nil t)
 (require 'gnus)
 (require 'gnus-int)
 (require 'gnus-sum)
diff --git a/lisp/registry.el b/lisp/registry.el
index cc03b20..edaa4f5 100644
--- a/lisp/registry.el
+++ b/lisp/registry.el
@@ -81,6 +81,8 @@
   (when (null (require 'ert nil t))
     (defmacro* ert-deftest (name () &body docstring-keys-and-body))))
 
+(require 'ert nil t)
+
 (eval-when-compile (require 'cl))
 (eval-and-compile
   (or (ignore-errors (progn

Re: no gnus registry new dependency on ert?

[David Engster]

Ted Zlatanov writes:
> On Wed, 06 Apr 2011 15:28:07 +0200 David Engster <deng@randomsample.de> wrote: 
>
> DE> It seems like putting (require 'ert) into an eval-when-compile is not
> DE> sufficient. You have to require it unconditionally.
>
> DE> So maybe your first idea was The Right Thing To Do, meaning to use
>
> DE> (require 'ert nil t)
>
> DE> and then test for feature 'ert. Alternatively, you could use the current
> DE> hack and put this require in there, too. But this is getting
> DE> increasingly ugly...
>
> See the attached patch, which just adds (require 'ert nil t) *in
> addition* to the defmacro hack.  It seems to DTRT for me, let me know
> what you think.

Yes, that's what I meant with the latter suggestion. I'm not sure if
this is the right way to solve this, but it works for me, too.

-David

Re: no gnus registry new dependency on ert?

[Ted Zlatanov]

On Wed, 06 Apr 2011 16:57:05 +0200 David Engster <deng@randomsample.de> wrote: 

DE> Ted Zlatanov writes:
>> See the attached patch, which just adds (require 'ert nil t) *in
>> addition* to the defmacro hack.  It seems to DTRT for me, let me know
>> what you think.

DE> Yes, that's what I meant with the latter suggestion. I'm not sure if
DE> this is the right way to solve this, but it works for me, too.

OK, it's pushed.  Let's hope it's the end of the hackery.

Ted

Re: no gnus registry new dependency on ert?

[David Engster]

David Engster writes:
> Ted Zlatanov writes:
>> On Wed, 06 Apr 2011 15:28:07 +0200 David Engster <deng@randomsample.de> wrote: 
>>
>> DE> It seems like putting (require 'ert) into an eval-when-compile is not
>
>> DE> sufficient. You have to require it unconditionally.
>>
>> DE> So maybe your first idea was The Right Thing To Do, meaning to use
>>
>> DE> (require 'ert nil t)
>>
>> DE> and then test for feature 'ert. Alternatively, you could use the current
>> DE> hack and put this require in there, too. But this is getting
>> DE> increasingly ugly...
>>
>> See the attached patch, which just adds (require 'ert nil t) *in
>> addition* to the defmacro hack.  It seems to DTRT for me, let me know
>> what you think.
>
> Yes, that's what I meant with the latter suggestion. I'm not sure if
> this is the right way to solve this, but it works for me, too.

Addendum: Maybe the "real" solution is to have the tests in a separate
file, so that a (require 'registry) won't instantiate the ERT tests. I
can see why having the tests in the file itself can be convenient, but
usually, test-suites are separated from the actual code? Anyway, I don't
really have a strong opinion on this.

-David

Re: no gnus registry new dependency on ert?

[Ted Zlatanov]

On Wed, 06 Apr 2011 17:03:50 +0200 David Engster <deng@randomsample.de> wrote: 

DE> Addendum: Maybe the "real" solution is to have the tests in a separate
DE> file, so that a (require 'registry) won't instantiate the ERT tests. I
DE> can see why having the tests in the file itself can be convenient, but
DE> usually, test-suites are separated from the actual code? Anyway, I don't
DE> really have a strong opinion on this.

I considered that approach but I think it's cleaner and easier to do it
in one file, if there are no technical reasons against it.

Ted