[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [pygame] 7 failures and 1 error in test suite



Hm...
the last attachment didn't make it in. here it is.



On Tue, 7 Jul 2009 13:28:18 +0200, <don@xxxxxxxxxxxxxxxxx> wrote:
> Hi,
> 
> I'm getting the feeling that either there are quite some bugs in the
> surface_test code or that I don't get what's going on.
> 
> first I looked at the failing test I mentioned before (~line 104):
> 
>     def make_surf(bpp, flags, masks):
>         pygame.Surface((10, 10), flags, bpp, masks)
>     self.failUnlessRaises(ValueError, make_surf, 32, 0,
>                           (0xFF000000, 0xFF0000, 0xFF00, 0))
> 
> and saw no good reason why the call to make_surf should raise an
> ValueError.
> Please tell me if I'm wrong here.
> 
> So then I thought maybe it's not a bug with 64-bit machines but rather
> with 32-bit ones and indeed I found a suspicious piece of code in base.c
> (line 335):
> 
>         *val = (Uint32) PyInt_AsLong (intobj);
> 
> if the intobj is negative (like 0xFF000000) the result won't be as
> desired.
> I changed that function (UintFromObj) to use a PyLong instead of a PyInt
> and then call PyLong_AsUnsignedLong (patch attached).
> 
> Again: If my assumption that the unit test should not raise that
> ValueError is wrong this might be a moot point.
> 
> with this patch applied the ValueError is not raised but then two other
> test in surface_test.py fail. namely test_blit_blend_rgba and
> test_blit_blend.
> 
> I first looked into test_blit_blend_rgba and found the function
> surf_get_masks to be questionable. the Py_BuildValue uses "i" as format
> characters which represent ints using "I" for unsigned int makes the Error
> go away (patch attached).
> 
> last but not least test_blit_blend. My suspicion is that the intify helper
> function screws things up. I also attached a patch with my idea of an
> intify function.
> 
> with all the patches applied all test pass except for test_flags (because
> of the UintFromObj.patch) so I also attached a patch which fixes the test
> (and renames it to test_masks because that is what is tested and not the
> flags.
> 
> 
> Note that I didn't test these patches on my 64-bit machine yet.
> I'll let you know as soon as I've done so.
> 
> Please let me know if I got something wrong.
> 
> 
> yours
> //Lorenz
> 
> 
> 
> On Tue, 07 Jul 2009 00:40:39 +0200, Lorenz Quack <don@xxxxxxxxxxxxxxxxx>
> wrote:
>> Hi,
>>
>> yup. the color test now pass.
>> the failing midi is not really a bug as I don't have portmidi.
>> I haven't looked into the failing surface test, yet.
>>
>>
>>
>> René Dudfield wrote:
>>> Hi,
>>>
>>> the Color 64bit errors are fixed.  A combination of testing against
>>> LONG_MAX, and using Uint32.
>>>
>>> Committed revision 2472.
>>>
>>> cheers,
>>>
>>>
>>> On Thu, Jul 2, 2009 at 11:52 PM, Hugo Arts <hugo.yoshi@xxxxxxxxx
>>> <mailto:hugo.yoshi@xxxxxxxxx>> wrote:
>>>
>>>     On Thu, Jul 2, 2009 at 2:55 PM, <don@xxxxxxxxxxxxxxxxx
>>>     <mailto:don@xxxxxxxxxxxxxxxxx>> wrote:
>>>      >
>>>      > Hi,
>>>      >
>>>      > On Thu, 2 Jul 2009 09:48:48 +1000, René Dudfield
>>>     <renesd@xxxxxxxxx <mailto:renesd@xxxxxxxxx>> wrote:
>>>      >> Cool, thanks for the testing.
>>>      >>
>>>      >>
>>>      >> On Thu, Jul 2, 2009 at 9:19 AM, Lorenz
>>>     Quack<don@xxxxxxxxxxxxxxxxx <mailto:don@xxxxxxxxxxxxxxxxx>> wrote:
>>>      >>> Ok, so here goes my analysis of the failures in ColorTypeTest:
>>>      >>> In src/color.c line 1435 (and some following lines) we bit
>>>     shift each
>>>      >> color
>>>      >>> component and stuff them
>>>      >>> in an unsigned long.
>>>      >>
>>>      >> Maybe this should be into a uint32 instead?
>>>      >>
>>>      >
>>>      > Yes, that would correspond to my solution 1) only better because
>>>     it ensures that it will have exactly 32 bit.
>>>      > BTW, are these types uint32, UInt8, ... in the C-standard?
>>>      >
>>>      > yours
>>>      > //Lorenz
>>>      >
>>>
>>>     No. ANSI C only defines the char, int, float, and double types,
>> along
>>>     with the short/long and signed/unsigned modifiers.
>>>     The sizes of these types are not defined in the standard. ANSI only
>>>     gives some minimum sizes, and mandates that
>>>
>>>     short int <= int <= long int
>>>     float <= double <= long double
>>>
>>>     So, in theory, the size of your int type is completely platform
>>>     dependent. In practice nearly every modern compiler uses the same
>>>     sizes,
>>>     char = 8, int = 32, float = 32, double = 64, but there are some
>>>     differences between 32bit and 64bit architectures (e.g. long int is
>>>     usually the same size as int on 32bit). Hence the often defined
>>>     uint32, uint8 etc. types.
>>>
>>>     See also http://en.wikipedia.org/wiki/C_data_types
>>>
>>>     Hugo
>>>
>>>

Attachment: test_flags.patch
Description: Binary data