Re: Unittest failure in python decoder tests

2008-10-07 Thread Gregory P. Smith

int vs long is a python 2.x annoyance.  It goes away in 3.x as the int
vs long distinction disappears.

Regardless, the correct thing to do in 2.x is to accept both int and
long types, this unit test is being too strict.  What matters is the
actual value, not the exact type.  In decoder_test.py
ReadScalarTestHelper line 107 the following assertion is too strict:

self.assert_(isinstance(result, type(expected_result)))

This should either be relaxed when type(expected_result) is int or
long, or you should add an additional parameter to the function
expected_result_types and use that as the second parameter to
isinstance (it can be a singluar type or a sequence of valid types).
Both int and long should be allowed.

-gps

On Mon, Oct 6, 2008 at 7:35 PM, Kenton Varda <[EMAIL PROTECTED]> wrote:
> [+petar]
> (Same problem as the other thread.)
>
> On Sun, Oct 5, 2008 at 2:41 AM, Iustin Pop <[EMAIL PROTECTED]> wrote:
>>
>> Hi there,
>>
>> Revision 50 changed the following line in
>> python/google/protobuf/internal/decoder_test.py:
>>
>> 122: ['sfixed32', decoder.Decoder.ReadSFixed32, -1,
>>  'ReadLittleEndian32', 0x],
>>
>> (in testReadScalars). The change is going from the above "-1" to
>> "long(-1)". While this passes ok on i386, it fails on amd64:
>>
>> i386: result is , expected is 
>> amd64: result is , expected is 
>>
>> I don't understand exactly what's going on, but I think the problem is
>> that an i386 system cannot represent that constant as int, but amd64 can
>> (not sure why...):
>> $ python32 -c 'print type(0x)'
>> 
>> $ python64 -c 'print type(0x)'
>> 
>>
>> Of course, just reverting the 'long(-1)' change makes the test fail on
>> i386.
>>
>> So, is this a genuine failure in the code, or is it just a bad unittest
>> that I can workaround by changing 0x to long(0x)?
>>
>> I'm thinking of applying this patch in order to make the tests pass on
>> both platforms:
>>
>> $ svn diff
>> Index: python/google/protobuf/internal/decoder_test.py
>> ===
>> --- python/google/protobuf/internal/decoder_test.py (revision 64)
>> +++ python/google/protobuf/internal/decoder_test.py (working copy)
>> @@ -120,7 +120,7 @@
>> ['fixed64', decoder.Decoder.ReadFixed64, 0x,
>> 'ReadLittleEndian64', 0x],
>> ['sfixed32', decoder.Decoder.ReadSFixed32, long(-1),
>> - 'ReadLittleEndian32', 0x],
>> + 'ReadLittleEndian32', long(0x)],
>> ['sfixed64', decoder.Decoder.ReadSFixed64, long(-1),
>>  'ReadLittleEndian64', 0x],
>> ['float', decoder.Decoder.ReadFloat, 0.0,
>>
>> regards,
>> iustin
>>
>>
>
>
> >
>

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Protocol Buffers" group.
To post to this group, send email to protobuf@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/protobuf?hl=en
-~--~~~~--~~--~--~---



Re: Unittest failure in python decoder tests

2008-10-07 Thread Pavel Shramov

On Sun, Oct 05, 2008 at 11:41:53AM +0200, Iustin Pop wrote:
> I don't understand exactly what's going on, but I think the problem is
> that an i386 system cannot represent that constant as int, but amd64 can
> (not sure why...):
> $ python32 -c 'print type(0x)'
> 
> $ python64 -c 'print type(0x)'
> 

>From python docs at [1]:
Plain integers (also just called integers) are implemented using long in
C, which gives them at least 32 bits of precision (sys.maxint is always
set to the maximum plain integer value for the current platform, the
minimum value is -sys.maxint - 1).

Since python lacks 'unsigned int' it's represented as long.

--
[1] http://www.python.org/doc/2.5.2/lib/typesnumeric.html

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Protocol Buffers" group.
To post to this group, send email to protobuf@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/protobuf?hl=en
-~--~~~~--~~--~--~---



Re: Unittest failure in python decoder tests

2008-10-07 Thread Pavel Shramov

On Sun, Oct 05, 2008 at 11:41:53AM +0200, Iustin Pop wrote:
> i386: result is , expected is 
> amd64: result is , expected is 
> 
> I don't understand exactly what's going on, but I think the problem is
> that an i386 system cannot represent that constant as int, but amd64 can
> (not sure why...):
> $ python32 -c 'print type(0x)'
> 
> $ python64 -c 'print type(0x)'
> 
Problem is in struct.unpack function
Unpacking "
$ python --version
Python 2.5.2

$ uname -m
x86_64
$ python -c 'import struct; print type(struct.unpack("
$ python --version
Python 2.5.2

Decode.ReadSFixed32 and InputStream.ReadLittleEndian32 don't check types
and pass value they got from struct.unpack. Possible fix will be
converting result of unpack function to fixed type (int or long).

Pavel

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Protocol Buffers" group.
To post to this group, send email to protobuf@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/protobuf?hl=en
-~--~~~~--~~--~--~---



Re: Unittest failure in python decoder tests

2008-10-06 Thread Kenton Varda
[+petar]
(Same problem as the other thread.)

On Sun, Oct 5, 2008 at 2:41 AM, Iustin Pop <[EMAIL PROTECTED]> wrote:

>
> Hi there,
>
> Revision 50 changed the following line in
> python/google/protobuf/internal/decoder_test.py:
>
> 122: ['sfixed32', decoder.Decoder.ReadSFixed32, -1,
>  'ReadLittleEndian32', 0x],
>
> (in testReadScalars). The change is going from the above "-1" to
> "long(-1)". While this passes ok on i386, it fails on amd64:
>
> i386: result is , expected is 
> amd64: result is , expected is 
>
> I don't understand exactly what's going on, but I think the problem is
> that an i386 system cannot represent that constant as int, but amd64 can
> (not sure why...):
> $ python32 -c 'print type(0x)'
> 
> $ python64 -c 'print type(0x)'
> 
>
> Of course, just reverting the 'long(-1)' change makes the test fail on
> i386.
>
> So, is this a genuine failure in the code, or is it just a bad unittest
> that I can workaround by changing 0x to long(0x)?
>
> I'm thinking of applying this patch in order to make the tests pass on
> both platforms:
>
> $ svn diff
> Index: python/google/protobuf/internal/decoder_test.py
> ===
> --- python/google/protobuf/internal/decoder_test.py (revision 64)
> +++ python/google/protobuf/internal/decoder_test.py (working copy)
> @@ -120,7 +120,7 @@
> ['fixed64', decoder.Decoder.ReadFixed64, 0x,
> 'ReadLittleEndian64', 0x],
> ['sfixed32', decoder.Decoder.ReadSFixed32, long(-1),
> - 'ReadLittleEndian32', 0x],
> + 'ReadLittleEndian32', long(0x)],
> ['sfixed64', decoder.Decoder.ReadSFixed64, long(-1),
>  'ReadLittleEndian64', 0x],
> ['float', decoder.Decoder.ReadFloat, 0.0,
>
> regards,
> iustin
>
> >
>

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Protocol Buffers" group.
To post to this group, send email to protobuf@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/protobuf?hl=en
-~--~~~~--~~--~--~---



Re: Unittest failure in python decoder tests

2008-10-06 Thread Iustin Pop

On Sun, Oct 05, 2008 at 02:48:56AM -0700, [EMAIL PROTECTED] wrote:
> 
> Wow we submitted the same bug 6 minutes off of each other.
> The test also passes with a fix by changing long(-1) to -1 on the line
> above.  I'm not sure which change is the 'correct' one.

Yes, but if you change long(-1) to -1, the test will fail (again) on
i386, so this is not a good way forward.

regards,
iustin

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Protocol Buffers" group.
To post to this group, send email to protobuf@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/protobuf?hl=en
-~--~~~~--~~--~--~---



Re: Unittest failure in python decoder tests

2008-10-05 Thread fpmchu

You're right.  I didn't read your email carefully.  This is indeed
strange behavior of Python.

Frank

On Oct 5, 3:05 am, Iustin Pop <[EMAIL PROTECTED]> wrote:
> On Sun, Oct 05, 2008 at 02:48:56AM -0700, [EMAIL PROTECTED] wrote:
>
> > Wow we submitted the same bug 6 minutes off of each other.
> > The test also passes with a fix by changing long(-1) to -1 on the line
> > above.  I'm not sure which change is the 'correct' one.
>
> Yes, but if you change long(-1) to -1, the test will fail (again) on
> i386, so this is not a good way forward.
>
> regards,
> iustin
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Protocol Buffers" group.
To post to this group, send email to protobuf@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/protobuf?hl=en
-~--~~~~--~~--~--~---



Re: Unittest failure in python decoder tests

2008-10-05 Thread fpmchu

Wow we submitted the same bug 6 minutes off of each other.
The test also passes with a fix by changing long(-1) to -1 on the line
above.  I'm not sure which change is the 'correct' one.

Frank

On Oct 5, 2:41 am, Iustin Pop <[EMAIL PROTECTED]> wrote:
> Hi there,
>
> Revision 50 changed the following line in
> python/google/protobuf/internal/decoder_test.py:
>
> 122: ['sfixed32', decoder.Decoder.ReadSFixed32, -1,
>       'ReadLittleEndian32', 0x],
>
> (in testReadScalars). The change is going from the above “-1” to
> “long(-1)”. While this passes ok on i386, it fails on amd64:
>
> i386: result is , expected is 
> amd64: result is , expected is 
>
> I don't understand exactly what's going on, but I think the problem is
> that an i386 system cannot represent that constant as int, but amd64 can
> (not sure why...):
> $ python32 -c 'print type(0x)'
> 
> $ python64 -c 'print type(0x)'
> 
>
> Of course, just reverting the 'long(-1)' change makes the test fail on
> i386.
>
> So, is this a genuine failure in the code, or is it just a bad unittest
> that I can workaround by changing 0x to long(0x)?
>
> I'm thinking of applying this patch in order to make the tests pass on
> both platforms:
>
> $ svn diff
> Index: python/google/protobuf/internal/decoder_test.py
> ===
> --- python/google/protobuf/internal/decoder_test.py     (revision 64)
> +++ python/google/protobuf/internal/decoder_test.py     (working copy)
> @@ -120,7 +120,7 @@
>          ['fixed64', decoder.Decoder.ReadFixed64, 0x,
>          'ReadLittleEndian64', 0x],
>          ['sfixed32', decoder.Decoder.ReadSFixed32, long(-1),
> -         'ReadLittleEndian32', 0x],
> +         'ReadLittleEndian32', long(0x)],
>          ['sfixed64', decoder.Decoder.ReadSFixed64, long(-1),
>           'ReadLittleEndian64', 0x],
>          ['float', decoder.Decoder.ReadFloat, 0.0,
>
> regards,
> iustin
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Protocol Buffers" group.
To post to this group, send email to protobuf@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/protobuf?hl=en
-~--~~~~--~~--~--~---