Patchwork util: add doctest to datestr()

login
register
mail settings
Submitter Adrian Buehlmann
Date April 8, 2016, 9:29 a.m.
Message ID <e4855d9b93ae1c1413b3.1460107760@kork>
Download mbox | patch
Permalink /patch/14428/
State Superseded
Commit 16255662446d2ce08bd0a4210df30afe3d6377f6
Delegated to: Yuya Nishihara
Headers show

Comments

Adrian Buehlmann - April 8, 2016, 9:29 a.m.
# HG changeset patch
# User Adrian Buehlmann <adrian@cadifra.com>
# Date 1460042647 -7200
# Node ID e4855d9b93ae1c1413b3ab67e9e605b968c89fa9
# Parent  ea86cdcd9b50bf38c6b9dd7bbaa04b9c8cc0aefb
util: add doctest to datestr()

The setlocale call is needed. If I leave it away, the active locale is -
surprisingly - *not* 'C' here.
Yuya Nishihara - April 8, 2016, 2:51 p.m.
On Fri, 08 Apr 2016 11:29:20 +0200, Adrian Buehlmann wrote:
> # HG changeset patch
> # User Adrian Buehlmann <adrian@cadifra.com>
> # Date 1460042647 -7200
> # Node ID e4855d9b93ae1c1413b3ab67e9e605b968c89fa9
> # Parent  ea86cdcd9b50bf38c6b9dd7bbaa04b9c8cc0aefb
> util: add doctest to datestr()
> 
> The setlocale call is needed. If I leave it away, the active locale is -
> surprisingly - *not* 'C' here.
> 
> diff --git a/mercurial/util.py b/mercurial/util.py
> --- a/mercurial/util.py
> +++ b/mercurial/util.py
> @@ -1576,7 +1576,22 @@
>      """represent a (unixtime, offset) tuple as a localized time.
>      unixtime is seconds since the epoch, and offset is the time zone's
>      number of seconds away from UTC. if timezone is false, do not
> -    append time zone to string."""
> +    append time zone to string.
> +
> +    >>> import locale
> +    >>> locale.setlocale(locale.LC_ALL, 'C')
> +    'C'

Perhaps test-doctest.py should do that globally? I got test failure on
Windows even without this patch.

> +    >>> datestr((0, False))

"offset" is integer, not bool.
Adrian Buehlmann - April 8, 2016, 3:58 p.m.
On 2016-04-08 16:51, Yuya Nishihara wrote:
> On Fri, 08 Apr 2016 11:29:20 +0200, Adrian Buehlmann wrote:
>> # HG changeset patch
>> # User Adrian Buehlmann <adrian@cadifra.com>
>> # Date 1460042647 -7200
>> # Node ID e4855d9b93ae1c1413b3ab67e9e605b968c89fa9
>> # Parent  ea86cdcd9b50bf38c6b9dd7bbaa04b9c8cc0aefb
>> util: add doctest to datestr()
>>
>> The setlocale call is needed. If I leave it away, the active locale is -
>> surprisingly - *not* 'C' here.
>>
>> diff --git a/mercurial/util.py b/mercurial/util.py
>> --- a/mercurial/util.py
>> +++ b/mercurial/util.py
>> @@ -1576,7 +1576,22 @@
>>      """represent a (unixtime, offset) tuple as a localized time.
>>      unixtime is seconds since the epoch, and offset is the time zone's
>>      number of seconds away from UTC. if timezone is false, do not
>> -    append time zone to string."""
>> +    append time zone to string.
>> +
>> +    >>> import locale
>> +    >>> locale.setlocale(locale.LC_ALL, 'C')
>> +    'C'
> 
> Perhaps test-doctest.py should do that globally? I got test failure on
> Windows even without this patch.
>

?

I'll probably stop digging this. In my experiments, setlocale in
test-doctest.py had no influence on the locale used inside the scope of
individual doctests.

Test passed here.

>> +    >>> datestr((0, False))
> 
> "offset" is integer, not bool.
> 

... which apparently doesn't match the spec of the function ("if
timezone is false, do not append time zone to string.")

Im dropping this patch anyway, as the patch by Florent now made its way.

Thanks for looking.
Yuya Nishihara - April 8, 2016, 4:23 p.m.
On Fri, 08 Apr 2016 17:58:16 +0200, Adrian Buehlmann wrote:
> On 2016-04-08 16:51, Yuya Nishihara wrote:
> > On Fri, 08 Apr 2016 11:29:20 +0200, Adrian Buehlmann wrote:  
> >> # HG changeset patch
> >> # User Adrian Buehlmann <adrian@cadifra.com>
> >> # Date 1460042647 -7200
> >> # Node ID e4855d9b93ae1c1413b3ab67e9e605b968c89fa9
> >> # Parent  ea86cdcd9b50bf38c6b9dd7bbaa04b9c8cc0aefb
> >> util: add doctest to datestr()
> >>
> >> The setlocale call is needed. If I leave it away, the active locale is -
> >> surprisingly - *not* 'C' here.
> >>
> >> diff --git a/mercurial/util.py b/mercurial/util.py
> >> --- a/mercurial/util.py
> >> +++ b/mercurial/util.py
> >> @@ -1576,7 +1576,22 @@
> >>      """represent a (unixtime, offset) tuple as a localized time.
> >>      unixtime is seconds since the epoch, and offset is the time zone's
> >>      number of seconds away from UTC. if timezone is false, do not
> >> -    append time zone to string."""
> >> +    append time zone to string.
> >> +
> >> +    >>> import locale
> >> +    >>> locale.setlocale(locale.LC_ALL, 'C')
> >> +    'C'  
> > 
> > Perhaps test-doctest.py should do that globally? I got test failure on
> > Windows even without this patch.
> >  
> 
> ?
> 
> I'll probably stop digging this. In my experiments, setlocale in
> test-doctest.py had no influence on the locale used inside the scope of
> individual doctests.
> 
> Test passed here.

I thought setlocale() is per process and test-doctest.py won't spawn new
process. But I didn't investigate it either.

> >> +    >>> datestr((0, False))  
> > 
> > "offset" is integer, not bool.
> >   
> 
> ... which apparently doesn't match the spec of the function ("if
> timezone is false, do not append time zone to string.")

It appears that the doc is wrong. There was "timezone" flag at 04c17fc39c84.
Adrian Buehlmann - April 8, 2016, 4:46 p.m.
On 2016-04-08 18:23, Yuya Nishihara wrote:
> On Fri, 08 Apr 2016 17:58:16 +0200, Adrian Buehlmann wrote:
>> On 2016-04-08 16:51, Yuya Nishihara wrote:
>>> On Fri, 08 Apr 2016 11:29:20 +0200, Adrian Buehlmann wrote:  
>>>> # HG changeset patch
>>>> # User Adrian Buehlmann <adrian@cadifra.com>
>>>> # Date 1460042647 -7200
>>>> # Node ID e4855d9b93ae1c1413b3ab67e9e605b968c89fa9
>>>> # Parent  ea86cdcd9b50bf38c6b9dd7bbaa04b9c8cc0aefb
>>>> util: add doctest to datestr()
>>>>
>>>> The setlocale call is needed. If I leave it away, the active locale is -
>>>> surprisingly - *not* 'C' here.
>>>>
>>>> diff --git a/mercurial/util.py b/mercurial/util.py
>>>> --- a/mercurial/util.py
>>>> +++ b/mercurial/util.py
>>>> @@ -1576,7 +1576,22 @@
>>>>      """represent a (unixtime, offset) tuple as a localized time.
>>>>      unixtime is seconds since the epoch, and offset is the time zone's
>>>>      number of seconds away from UTC. if timezone is false, do not
>>>> -    append time zone to string."""
>>>> +    append time zone to string.
>>>> +
>>>> +    >>> import locale
>>>> +    >>> locale.setlocale(locale.LC_ALL, 'C')
>>>> +    'C'  
>>>
>>> Perhaps test-doctest.py should do that globally? I got test failure on
>>> Windows even without this patch.
>>>  
>>
>> ?
>>
>> I'll probably stop digging this. In my experiments, setlocale in
>> test-doctest.py had no influence on the locale used inside the scope of
>> individual doctests.
>>
>> Test passed here.
> 
> I thought setlocale() is per process and test-doctest.py won't spawn new
> process. But I didn't investigate it either.

To me, it looks like each doctest-block has a new locale-scope (per
function). Could be similar to the fact that each one has its own
globals. I might be wrong though.

No matter what I tried: the locale was correctly set only if I did set
it in the doctest block. Which is quite surprising.

>>>> +    >>> datestr((0, False))  
>>>
>>> "offset" is integer, not bool.
>>>   
>>
>> ... which apparently doesn't match the spec of the function ("if
>> timezone is false, do not append time zone to string.")
> 
> It appears that the doc is wrong. There was "timezone" flag at 04c17fc39c84.
>

Patch

diff --git a/mercurial/util.py b/mercurial/util.py
--- a/mercurial/util.py
+++ b/mercurial/util.py
@@ -1576,7 +1576,22 @@ 
     """represent a (unixtime, offset) tuple as a localized time.
     unixtime is seconds since the epoch, and offset is the time zone's
     number of seconds away from UTC. if timezone is false, do not
-    append time zone to string."""
+    append time zone to string.
+
+    >>> import locale
+    >>> locale.setlocale(locale.LC_ALL, 'C')
+    'C'
+    >>> datestr((0, False))
+    'Thu Jan 01 00:00:00 1970 +0000'
+    >>> datestr((42, False))
+    'Thu Jan 01 00:00:42 1970 +0000'
+    >>> datestr((-42, False))
+    'Thu Jan 01 00:00:00 1970 +0000'
+    >>> datestr((0x7fffffff, False))
+    'Tue Jan 19 03:14:07 2038 +0000'
+    >>> datestr((-0x7fffffff, False))
+    'Thu Jan 01 00:00:00 1970 +0000'
+    """
     t, tz = date or makedate()
     if t < 0:
         t = 0   # time.gmtime(lt) fails on Windows for lt < -43200