bug#24944: [PATCH] Skip test-seq-random-elt-take-all on Emacs 24

bug#24944: [PATCH] Skip test-seq-random-elt-take-all on Emacs 24

[Sean Whitton]

The function `map-length' is not available.
---
 packages/seq/tests/seq-tests.el | 1 +
 1 file changed, 1 insertion(+)

diff --git a/packages/seq/tests/seq-tests.el b/packages/seq/tests/seq-tests.el
index 53930fa..7a9a452 100644
--- a/packages/seq/tests/seq-tests.el
+++ b/packages/seq/tests/seq-tests.el
@@ -351,6 +351,7 @@ Evaluate BODY for each created sequence.
                    ["xxx" "xx" "x"]))))
 
 (ert-deftest test-seq-random-elt-take-all ()
+  (skip-unless (version> emacs-version "25"))
   (let ((seq '(a b c d e))
         (count '()))
     (should (= 0 (map-length count)))
-- 
2.10.2

bug#24944: [PATCH] Skip test-seq-random-elt-take-all on Emacs 24

[Eli Zaretskii]

> From: Sean Whitton <spwhitton@spwhitton.name>
> Date: Mon, 14 Nov 2016 21:00:50 -0700
> Cc: Sean Whitton <spwhitton@spwhitton.name>
> 
> The function `map-length' is not available.
> ---
>  packages/seq/tests/seq-tests.el | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/packages/seq/tests/seq-tests.el b/packages/seq/tests/seq-tests.el
> index 53930fa..7a9a452 100644
> --- a/packages/seq/tests/seq-tests.el
> +++ b/packages/seq/tests/seq-tests.el
> @@ -351,6 +351,7 @@ Evaluate BODY for each created sequence.
>                     ["xxx" "xx" "x"]))))
>  
>  (ert-deftest test-seq-random-elt-take-all ()
> +  (skip-unless (version> emacs-version "25"))
>    (let ((seq '(a b c d e))
>          (count '()))
>      (should (= 0 (map-length count)))

A better test is to use fboundp (after requiring the package that
should define it) to probe whether the function exists or not, instead
of relying on the version.

Thanks.

bug#24944: [PATCH] Skip test-seq-random-elt-take-all on Emacs 24

[Sean Whitton]

[-- Attachment #1.1: Type: text/plain, Size: 370 bytes --]

On Tue, Nov 15, 2016 at 05:20:36PM +0200, Eli Zaretskii wrote:
> A better test is to use fboundp (after requiring the package that
> should define it) to probe whether the function exists or not, instead
> of relying on the version.

Thanks.  Improved patch attached.

Note that this is against the version of seq in elpa.git, not emacs.git

-- 
Sean Whitton

[-- Attachment #1.2: 0001-Skip-test-seq-random-elt-take-all-on-Emacs-24.patch --]
[-- Type: text/x-diff, Size: 1040 bytes --]

From 494e3476538cd2cefdc7343057ccfb33412aca73 Mon Sep 17 00:00:00 2001
From: Sean Whitton <spwhitton@spwhitton.name>
Date: Mon, 14 Nov 2016 20:58:14 -0700
Subject: [PATCH] Skip test-seq-random-elt-take-all on Emacs 24

The function `map-length' is not available.
---
 packages/seq/tests/seq-tests.el | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/packages/seq/tests/seq-tests.el b/packages/seq/tests/seq-tests.el
index 53930fa..0e15f25 100644
--- a/packages/seq/tests/seq-tests.el
+++ b/packages/seq/tests/seq-tests.el
@@ -28,6 +28,7 @@
 
 (require 'ert)
 (require 'seq)
+(require 'map)
 
 (defmacro with-test-sequences (spec &rest body)
   "Successively bind VAR to a list, vector, and string built from SEQ.
@@ -351,6 +352,7 @@ Evaluate BODY for each created sequence.
                    ["xxx" "xx" "x"]))))
 
 (ert-deftest test-seq-random-elt-take-all ()
+  (skip-unless (fboundp 'map-length))
   (let ((seq '(a b c d e))
         (count '()))
     (should (= 0 (map-length count)))
-- 
2.10.2


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

bug#24944: [PATCH] Skip test-seq-random-elt-take-all on Emacs 24

[Nicolas Petton]

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

Sean Whitton <spwhitton@spwhitton.name> writes:

> Thanks.  Improved patch attached.

Looks good, but what about not using map.el here (it's an alist) instead
of skipping the test?

Cheers,
Nico

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

bug#24944: [PATCH] Skip test-seq-random-elt-take-all on Emacs 24

[Sean Whitton]

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

On Tue, Nov 15, 2016 at 06:02:56PM +0100, Nicolas Petton wrote:
> Sean Whitton <spwhitton@spwhitton.name> writes:
> 
> > Thanks.  Improved patch attached.
> 
> Looks good, but what about not using map.el here (it's an alist) instead
> of skipping the test?

Are you suggesting: s/map-length/length/ ?

-- 
Sean Whitton

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

bug#24944: [PATCH] Skip test-seq-random-elt-take-all on Emacs 24

[Nicolas Petton]

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

Sean Whitton <spwhitton@spwhitton.name> writes:

> Are you suggesting: s/map-length/length/ ?

There's also a usage of `map-put' to replace, as well as the `require'
in the beginning of the file.

However, seq.el was introduced in Emacs 25.1, so in the Emacs repository
there is no need to change anything there.  I suppose you're talking
about the version of seq.el in ELPA?

Cheers,
Nico

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

bug#24944: [PATCH] Skip test-seq-random-elt-take-all on Emacs 24

[Sean Whitton]

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

Hello,

On Tue, Nov 15, 2016 at 09:13:26PM +0100, Nicolas Petton wrote:
> Sean Whitton <spwhitton@spwhitton.name> writes:
> 
> > Are you suggesting: s/map-length/length/ ?
> 
> There's also a usage of `map-put' to replace, as well as the `require'
> in the beginning of the file.

Can the usage of `map-put' be replaced?

If not, it might be best just to wrap both the `require' and the whole
test in my original version check.

> However, seq.el was introduced in Emacs 25.1, so in the Emacs
> repository there is no need to change anything there.  I suppose
> you're talking about the version of seq.el in ELPA?

Yes.

For background, I was prompted to write this patch while updating the
packaging of seq.el in Debian, which for the moment contains both
emacs24 and emacs25.

Thanks!

-- 
Sean Whitton

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

bug#24944: [PATCH] Skip test-seq-random-elt-take-all on Emacs 24

[Nicolas Petton]

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

Sean Whitton <spwhitton@spwhitton.name> writes:

> Can the usage of `map-put' be replaced?

Yes, I can install a patch if you want.

> For background, I was prompted to write this patch while updating the
> packaging of seq.el in Debian, which for the moment contains both
> emacs24 and emacs25.

Ok, I get it now :)

Nico

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

bug#24944: [PATCH] Skip test-seq-random-elt-take-all on Emacs 24

[Sean Whitton]

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

On Tue, Nov 15, 2016 at 11:58:19PM +0100, Nicolas Petton wrote:
> Sean Whitton <spwhitton@spwhitton.name> writes:
> 
> > Can the usage of `map-put' be replaced?
> 
> Yes, I can install a patch if you want.

That would be great, but obviously not a priority.  Thanks again for
your feedback.

-- 
Sean Whitton

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

bug#24944: [PATCH] Skip test-seq-random-elt-take-all on Emacs 24

[Nicolas Petton]

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

Sean Whitton <spwhitton@spwhitton.name> writes:

> That would be great, but obviously not a priority.  Thanks again for
> your feedback.

I installed a patch in Emacs and ELPA (commit 2d6022a).  I'm closing the
issue.

Cheers,
Nico

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]