On Thu Oct 23 01:38:59 2008, mgrimes wrote: > Christoph, > > Thanks for your help. This has been a great, low intensity, way to > learn a bit of parrot. > I think I have addressed everything, and I have attached a new patch. > > > The patch no longer applies cleanly to objects.t, and I thought > it'd be > > better to let you merge the recent changes from svn and add the > .pcc_sub > > Looks like there were just a few changes to spacing. This patch > applied cleanly > to version 32115. I added the .pcc_sub tests to objects.t. > > > It's a good idea to test for exception types rather than exact > messages. This > > keeps the tests passing if the wording of the message is changed. > The > > exception type is much more likely to remain constant. This > recommended but > > not a blocker. > > I wanted to keep the changes to the code down to a minimum, so I was > reluctant > to add ExecptionHandler objects. If there is a simpler way (ie, > testing with > typeof, etc), I would be happy to look into changing it. > > > Test messages for shouldn't be blank. If a test fails, it should be > fairly > > Oops. Got a bit ahead of myself with complex.t. Messages have been > added. > > > This is a minor nit, but "-ve" and "+ve" generally expand to > "negative" and > > Ah... should have known. Thanks. > > > Again, thanks for working on this. > > Happy to help. Thanks to you and the rest of the team for doing the > heavy lifting! > > -Mark
Good times. I changed the register names to use the $P0 format, switched a few tests to isa_ok and made some minor changes (description updates, removing unneeded code/comments). The getting_null_attribute test was only throwing an exception because the old version was trying to print a null PMC. It tests nullness more sanely now. The exception-throwing tests are a little more defensive now. This doesn't matter when they pass but should make debugging easier if they ever fail. pmichaud confirmed that the typeof_class test tests the right thing, so I enabled that one too. Apart from those (mostly cosmetic) changes, the patch was good. It was committed as r32145 and r32149 after I checked that everything was sane and no tests got dropped in the conversion. Thanks.
