[PATCH] Fix tramp test

[PATCH] Fix tramp test

[Przemysław Wojnowski]

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

Hello everybody,

A tiny patch for a test that fails in non English systems. OS output summary is
language dependent and for example in Polish it displays "razem 1234" instead of
"total 1234".

Please let me know if it can't be applied for some reason.

Thanks!

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-tramp-test.patch --]
[-- Type: text/x-patch; name="0001-Fix-tramp-test.patch", Size: 1407 bytes --]

From 214424ee32c7723e34924b220eb9f9a33881f9f0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Przemys=C5=82aw=20Wojnowski?= <esperanto@cumego.com>
Date: Sun, 22 Feb 2015 13:12:02 +0100
Subject: [PATCH] Fix tramp test.

Make summary line text matching working in non English systems too.
---
 test/ChangeLog                | 4 ++++
 test/automated/tramp-tests.el | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/test/ChangeLog b/test/ChangeLog
index 656124e..44e9444 100644
--- a/test/ChangeLog
+++ b/test/ChangeLog
@@ -1,5 +1,9 @@
 2015-02-22  Przemysław Wojnowski  <esperanto@cumego.com>
 
+	* automated/tramp-tests.el (tramp-test17-insert-directory):
+	Fix test. Make summary line text matching working in non English
+	systems too.
+
 	* automated/cl-lib-tests.el: New tests.
 	(cl-digit-char-p): Check returned value.
 
diff --git a/test/automated/tramp-tests.el b/test/automated/tramp-tests.el
index 2c4610c..e60a13f 100644
--- a/test/automated/tramp-tests.el
+++ b/test/automated/tramp-tests.el
@@ -955,7 +955,7 @@ This tests also `file-directory-p' and `file-accessible-directory-p'."
 	     (looking-at-p
 	      (concat
 	       ;; There might be a summary line.
-	       "\\(total.+[[:digit:]]+\n\\)?"
+	       "\\([[:alpha:]].+[[:digit:]]+\n\\)?"
 	       ;; We don't know in which order "." and ".." appear.
 	       "\\(.+ \\.?\\.\n\\)\\{2\\}"
 	       ".+ foo$")))))
-- 
2.1.0

Re: [PATCH] Fix tramp test

[Michael Albinus]

Przemysław Wojnowski <esperanto@cumego.com> writes:

> Hello everybody,
>
> A tiny patch for a test that fails in non English systems. OS output summary is
> language dependent and for example in Polish it displays "razem 1234" instead of
> "total 1234".
>
> Please let me know if it can't be applied for some reason.
>
> diff --git a/test/automated/tramp-tests.el b/test/automated/tramp-tests.el
> index 2c4610c..e60a13f 100644
> --- a/test/automated/tramp-tests.el
> +++ b/test/automated/tramp-tests.el
> @@ -955,7 +955,7 @@ This tests also `file-directory-p' and `file-accessible-directory-p'."
>  	     (looking-at-p
>  	      (concat
>  	       ;; There might be a summary line.
> -	       "\\(total.+[[:digit:]]+\n\\)?"
> +	       "\\([[:alpha:]].+[[:digit:]]+\n\\)?"

Better might be "\\([[:alpha:]]+[[:blank:]]+[[:digit:]]+\n\\)?"

>  	       ;; We don't know in which order "." and ".." appear.
>  	       "\\(.+ \\.?\\.\n\\)\\{2\\}"
>  	       ".+ foo$")))))

Beside of this, it looks OK to me. Please apply.

> Thanks!

Best regards, Michael.

Re: [PATCH] Fix tramp test

[Eli Zaretskii]

> From: Michael Albinus <michael.albinus@gmx.de>
> Date: Sun, 22 Feb 2015 15:14:57 +0100
> Cc: emacs-devel@gnu.org
> 
> > A tiny patch for a test that fails in non English systems. OS output summary is
> > language dependent and for example in Polish it displays "razem 1234" instead of
> > "total 1234".
> >
> > Please let me know if it can't be applied for some reason.
> >
> > diff --git a/test/automated/tramp-tests.el b/test/automated/tramp-tests.el
> > index 2c4610c..e60a13f 100644
> > --- a/test/automated/tramp-tests.el
> > +++ b/test/automated/tramp-tests.el
> > @@ -955,7 +955,7 @@ This tests also `file-directory-p' and `file-accessible-directory-p'."
> >  	     (looking-at-p
> >  	      (concat
> >  	       ;; There might be a summary line.
> > -	       "\\(total.+[[:digit:]]+\n\\)?"
> > +	       "\\([[:alpha:]].+[[:digit:]]+\n\\)?"
> 
> Better might be "\\([[:alpha:]]+[[:blank:]]+[[:digit:]]+\n\\)?"

IMO, [:alpha:] is too indiscreet: it will match almost anything,
except in pure ASCII range.  Including some error message, for
example.

So I think it would be much better to keep the literal "total", and
instead set LC_MESSAGES=en_US.UTF-8, or something similar.

Re: [PATCH] Fix tramp test

[Michael Albinus]

Eli Zaretskii <eliz@gnu.org> writes:

>> Better might be "\\([[:alpha:]]+[[:blank:]]+[[:digit:]]+\n\\)?"
>
> IMO, [:alpha:] is too indiscreet: it will match almost anything,
> except in pure ASCII range.  Including some error message, for
> example.
>
> So I think it would be much better to keep the literal "total", and
> instead set LC_MESSAGES=en_US.UTF-8, or something similar.

Tramp sets LC_ALL to "en_US.utf8", "C.utf8", or "C". In this order,
whatever is available on the remote system. But a user is free to
overwrite such settings.

Best regards, Michael.

Re: [PATCH] Fix tramp test

[Eli Zaretskii]

> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: esperanto@cumego.com,  emacs-devel@gnu.org
> Date: Sun, 22 Feb 2015 17:25:14 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> Better might be "\\([[:alpha:]]+[[:blank:]]+[[:digit:]]+\n\\)?"
> >
> > IMO, [:alpha:] is too indiscreet: it will match almost anything,
> > except in pure ASCII range.  Including some error message, for
> > example.
> >
> > So I think it would be much better to keep the literal "total", and
> > instead set LC_MESSAGES=en_US.UTF-8, or something similar.
> 
> Tramp sets LC_ALL to "en_US.utf8", "C.utf8", or "C". In this order,
> whatever is available on the remote system. But a user is free to

Not sure what scenario are we talking about here.  Do you mean that
Tramp sets LC_ALL, but the user overrides that with LC_MESSAGES?

Or do you mean that you want to cater to users who have neither
en_US.utf8 nor C.utf8 installed?

In any case, I meant to set that as part of the test in question, not
in general.

Re: [PATCH] Fix tramp test

[Michael Albinus]

Eli Zaretskii <eliz@gnu.org> writes:

>> Tramp sets LC_ALL to "en_US.utf8", "C.utf8", or "C". In this order,
>> whatever is available on the remote system. But a user is free to
>
> Not sure what scenario are we talking about here.  Do you mean that
> Tramp sets LC_ALL, but the user overrides that with LC_MESSAGES?

Yes.

> In any case, I meant to set that as part of the test in question, not
> in general.

Yep. I've committed a patch to the master which does this. It is not
perfect (there might be problems when tramp-test-temporary-file-directory
points to a real remote directory), but for the time being it is sufficient.

Best regards, Michael.

Re: [PATCH] Fix tramp test

[Przemysław Wojnowski]

It works. Thanks!

Re: [PATCH] Fix tramp test

[Andreas Schwab]

Michael Albinus <michael.albinus@gmx.de> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> Tramp sets LC_ALL to "en_US.utf8", "C.utf8", or "C". In this order,
>>> whatever is available on the remote system. But a user is free to
>>
>> Not sure what scenario are we talking about here.  Do you mean that
>> Tramp sets LC_ALL, but the user overrides that with LC_MESSAGES?
>
> Yes.

Note that LC_ALL has higher priority than LC_MESSAGES.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] Fix tramp test

[Michael Albinus]

Andreas Schwab <schwab@linux-m68k.org> writes:

>>> Not sure what scenario are we talking about here.  Do you mean that
>>> Tramp sets LC_ALL, but the user overrides that with LC_MESSAGES?
>>
>> Yes.
>
> Note that LC_ALL has higher priority than LC_MESSAGES.

Don't know what a user does. Instead of setting LC_MESSAGES, she could
also set LC_ALL. That's why I fix it in the Tramp test; if a user
overwrites Tramp settings with something private, it's not Tramp's
problem(*).

> Andreas.

Best regards, Michael.

(*): Usually it is my problem then, handling bug reports ...

Re: [PATCH] Fix tramp test

[Eli Zaretskii]

> Date: Sun, 22 Feb 2015 18:00:44 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: esperanto@cumego.com, emacs-devel@gnu.org
> 
> IMO, [:alpha:] is too indiscreet

I mean "indiscriminate", sorry for an embarrassing typo.