On 17.05.2022 16:29, Rick McGuire wrote:
If you're playing around with this, also check the FORWARD instruction tests to see if they need adjustment. If there's not good coverage for these situation, then additional tests should be added.

Also, there need to be tests for the FORWARD instruction where it is used to pass the method invocation to a different object with a superclass override to make sure that works.

Will do (would not be able to do them today, urgent need to do work related 
stuff).

---rony



On Tue, May 17, 2022 at 10:10 AM Rony G. Flatscher <rony.flatsc...@wu.ac.at> 
wrote:

    So this is the patch I intend to apply such that message instructions 
report exactly the same
    errors as .Message and .Object:

        Index: interpreter/expression/ExpressionMessage.cpp
        ===================================================================
        --- interpreter/expression/ExpressionMessage.cpp        (revision 12394)
        +++ interpreter/expression/ExpressionMessage.cpp        (working copy)
        @@ -162,6 +162,13 @@
              if (super != OREF_NULL)
              {
                  _super = (RexxClass *)super->evaluate(context, stack);
        +        // _super an instance of TheClassClass
        +        if (!_super->isInstanceOf(TheClassClass))
        +        {
        +            reportException(Error_Invalid_argument_noclass, "SCOPE", 
"Class");
        +        }
        +        // validate the starting scope
        +        _target->validateScopeOverride(_super);
                  // we send the message using the stack, which
                  // expects to find the target and the arguments
                  // on the stack, but not the super.  We need to
        Index: interpreter/instructions/MessageInstruction.cpp
        ===================================================================
        --- interpreter/instructions/MessageInstruction.cpp     (revision 12394)
        +++ interpreter/instructions/MessageInstruction.cpp     (working copy)
        @@ -161,6 +161,13 @@
              {
                  // get the superclass target
                  _super = (RexxClass *)super->evaluate(context, stack);
        +        // _super an instance of TheClassClass
        +        if (!_super->isInstanceOf(TheClassClass))
        +        {
        +            reportException(Error_Invalid_argument_noclass, "SCOPE", 
"Class");
        +        }
        +        // validate the starting scope
        +        _target->validateScopeOverride(_super);
                  // we send the message using the stack, which
                  // expects to find the target and the arguments
                  // on the stack, but not the super.  We need to

    ---rony


    On 17.05.2022 15:46, Rony G. Flatscher wrote:

    Forgot to add tests for the message instructions so went back to the 
testgroup to add them
    and found out, that the tests for illegal overrides returned 97.1 and not 
what .Message and
    .Object raise, namely 93.957 (receiver class not a subclass of override 
object) and 88.914
    (SCOPE must be instance of class Class).

    Instead of having 97.1 (object method not found), which does not point at 
the reason I would
    like a message instructions to report an error with the override 
explicitly. Of course, the
    tests depend on whether 97.1 or 93.957 gets reported.

    To do so I intend to add this:

        Index: interpreter/expression/ExpressionMessage.cpp
        ===================================================================
        --- interpreter/expression/ExpressionMessage.cpp        (revision 12394)
        +++ interpreter/expression/ExpressionMessage.cpp        (working copy)
        @@ -162,6 +162,8 @@
              if (super != OREF_NULL)
              {
                  _super = (RexxClass *)super->evaluate(context, stack);
        +        // validate the starting scope
        +        _target->validateScopeOverride(_super);
                  // we send the message using the stack, which
                  // expects to find the target and the arguments
                  // on the stack, but not the super.  We need to
        Index: interpreter/instructions/MessageInstruction.cpp
        ===================================================================
        --- interpreter/instructions/MessageInstruction.cpp     (revision 12394)
        +++ interpreter/instructions/MessageInstruction.cpp     (working copy)
        @@ -161,6 +161,8 @@
              {
                  // get the superclass target
                  _super = (RexxClass *)super->evaluate(context, stack);
        +        // validate the starting scope
        +        _target->validateScopeOverride(_super);
                  // we send the message using the stack, which
                  // expects to find the target and the arguments
                  // on the stack, but not the super.  We need to

    Maybe a test whether _super is an instance of class Class should be carried 
out first to
    become able to also raise 88.914? If so, what would be the easiest way to 
do so?

    Am I missing something else? Are there any objections?

    ---rony


    On 17.05.2022 14:03, Rick McGuire wrote:
    I repeat, these are not acceptable tests. Please make the appropriate 
corrections to them.
    Turn these into actual functional tests, otherwise they have no real 
purpose.

    Rick

    On Tue, May 17, 2022 at 8:01 AM Rony G. Flatscher <rony.flatsc...@wu.ac.at> 
wrote:

        On 17.05.2022 13:37, Rick McGuire wrote:
        This is not an acceptable way to fix these tests. Just removing the 
expected error and
        adding a totally unnecessary tautological assertion is not enough. 
These tests need to
        also verify that the correct method has been invoked by checking the 
return value from
        the method call.

        Two remarks:

          * There were existing tests that expected an error, if the override 
was not done to a
            message to self. These tests would now fail as these overrides are 
allowed. So
            removing the expected error turns the test into the opposite, 
testing whether an
            override is accepted and carried out. If the override takes place 
successfully
            assertTrue(.true) is used to increase the success assertion 
counter, otherwise the
            test suite would not be able to increase that counter anymore.

          * Ad testing whether the overrides work correctly, i.e. invoking the 
expected methods,
            these tests are the ones that I added explicitly, such that this 
aspect gets tested
            as well for send, sendWith, start, startWith for both, the .Message 
and the .Object
            classes. If you look up these test groups you will see that the 
tests include
            override tests for mixinclasses where the results of the invoked 
messages get tested
            for correctness. It may be the case that I am missing some tests, 
if so, please advise.

        ---rony


_______________________________________________
Oorexx-devel mailing list
Oorexx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/oorexx-devel

Reply via email to