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.
Rick
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
>
_______________________________________________
Oorexx-devel mailing list
Oorexx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/oorexx-devel
