[perl #60600] [PATCH] rewrite of ops.t to PIR

[Christoph Otto via RT]

On Sun Nov 16 19:47:36 2008, stockwellb wrote:
> rewrite of t/oo/ops.t to PIR.
>
> ops.t | 265
> ++++++++++++++++++++++++++++++++----------------------------------
> 1 file changed, 130 insertions(+), 135 deletions(-)
>

In op_get_class_p_p, it looks like you switch from "Ape" to "Monkey"
when getting the class from a namespace. I can't picture this causing
any problems, but it's a good idea to keep the subs as self-contained as
possible. As changing "Monkey" to "Ape" in that sub doesn't cause any
failures, is there any reason not to do so?

Also, tests should be very explicit about which exception type(s)
they're catching. This keeps other incidental exceptions from masking
bugs. The first test in t/pmc/ro.t is a good example of what to do.
You can find the exception type by acking Parrot for the exception's
message.

Other than that, the patch looks good. Make those changes and I'll be
glad to apply it.

Christoph
_______________________________________________
http://lists.parrot.org/mailman/listinfo/parrot-dev

[Christoph Otto via RT]

On Tue Nov 18 03:40:31 2008, stockwellb wrote:

> On Mon, Nov 17, 2008 at 3:23 PM, Christoph Otto via RT <
> parrotbug...@parrotcode.org> wrote:
>
> > On Sun Nov 16 19:47:36 2008, stockwellb wrote:
> > > rewrite of t/oo/ops.t to PIR.
> > >
> > > ops.t | 265
> > > ++++++++++++++++++++++++++++++++----------------------------------
> > > 1 file changed, 130 insertions(+), 135 deletions(-)
> > >
> >
> > In op_get_class_p_p, it looks like you switch from "Ape" to "Monkey"
> > when getting the class from a namespace. I can't picture this causing
> > any problems, but it's a good idea to keep the subs as self-contained as
> > possible. As changing "Monkey" to "Ape" in that sub doesn't cause any
> > failures, is there any reason not to do so?
>
>

> Yes. Monkey is already a registered class in an earlier test sub.
> Namespaces seemed like overkill so I just used another Class name.

>
> >
> >
> > Also, tests should be very explicit about which exception type(s)
> > they're catching. This keeps other incidental exceptions from masking
> > bugs. The first test in t/pmc/ro.t is a good example of what to do.
> > You can find the exception type by acking Parrot for the exception's
> > message.
>
>

> Thanks for the help on this one. I hadn't done anything with
> ExceptionHandlers yet. I removed several error traps as they were general.
> The remaining one is now trapping for an explicit error.

>
> >
> >
> > Other than that, the patch looks good. Make those changes and I'll be
> > glad to apply it.
>
>

> I'm attaching the diff. I hope this is the correct way.
> ops.t | 219
> +++++++++++++++++++++++-------------------------------------------
> 1 file changed, 78 insertions(+), 141 deletions(-)
>
>
>
> >
> >
> > Christoph
> >
>
>
>

For future reference, please give different versions of a patch
different names, e.g. ops_test_rewrite.patch, ops_test_rewrite_v2.patch,
etc so it's easy to tell which version should be applied.

The updated patch was applied in r32809. Thanks for contributing.

Christoph
_______________________________________________
http://lists.parrot.org/mailman/listinfo/parrot-dev