Test Case keeps passing even if tested class is removed from system

[Bajger]

Bug description A subclass of TestCase with test method that refer to some class and tested method keeps passing, even if tested class has been removed previously from the system.

To Reproduce Steps to reproduce the behavior:

1. Create new Class and method, e.g.: Aclass>>foo that returns empty string
2. Jump to test class and implement test method testFoo
3. Write in test method e.g. : `self assert: Aclass new foo equals: ''
4. Run test (shold pass)
5. Remove Aclass class from menu of system browser
6. Re-run test -> keeps passing

Expected behavior Test should end up with error when tested class is removed from the system.

Screenshots

Version information:

• OS: MacOS Monterey
• Version: 12.2.1
• Pharo Version P11, P10

Expected development cost Unknown.

[MarcusDenker]

Yes, this is expected: we can not re-scan the whole system and re-compile the methods when we delete a class (we did in the past, and it was too slow).

Thus what happens is that the method continues to reference the deleted class (we call these classes "obsolete"). And thus the test passes, as this class is just a normal class with the name changed to "AnObsoleteAClass" which is not referenced in the global dictionary.

[MarcusDenker]

We should think again if we can do something more clever... e.g. can we turn the variable into an Undeclared without having to recompile?

(Sometimes I think that in the end, these are all hints that we should late-bind globals vars more)

[Bajger]

@MarcusDenker: It is quite weird that system is in different state than before creation of method (after deletion it give instance of obsolete class, but before creation it is nil). Even if I recompile test method (with altering original test code) and leave undeclared reference there, it still gives original assertion (state before deletion of a class). So recompiling test method that refer to undeclared class didn't really help.

[MarcusDenker]

If it is recompiled to an Undeclared, the Variable that references the class should read as "nil"... and in this special case, the DNU exception handler should open a dialog that asks to define a class (an improvement we made to simplify test-driven development).

[Bajger]

If it is recompiled to an Undeclared, the Variable that references the class should read as "nil"..

Sorry I wrote my prior statement confused way: I wanted to say, after the removal and recompiling test method, test method temp variable still refers to obsolete class instance. So assertion gives a wrong result in this case. I have no experience, so I don't know how to implement your proposal: "e.g. can we turn the variable into an Undeclared without having to recompile?" This should handle class deletion method?

[MarcusDenker]

instances references by running code while the class is deleted will always have to reference the instance that is now obsolete. That is what obsolete classes are: classes that have been deleted but not yet GCed because someone references them.

[Bajger]

To keep momentum on this, what would be the good approach to resolve? e.g.: In deletion (of class) method, when referenced objects becoming AnObsoleteClass, can we check if reference is in test case method and recompile test method with Undeclared? Is it supposed to be too slow (depending on nr. of test methods)?

[MarcusDenker]

What we did in the past was to do that: when a class go deleted, we checked all the references to that class and re-compiled the methods.

The problem with that is that it was just too slow.

• original issue tracker entry: Huge performances regression on package removal #2369
• The fix we did: Speedup-class-removal #6816

So this means we tried to do what you suggests in the past, but it was just far too slow.

To fix this for real, we have to radically re-think how Undeclared Variables work (I think).

[MarcusDenker]

Ups, no, we did not recompile: Just checking if a class is still used (that is, finding the first method accessing it) was too slow.

We now add the class to Undeclared always, thus the references to a deleted class are Undeclared, but the Undeclated continues to reference the Obsolete class, which is consistent to any other variable referencing it.

E.g. imagine you have some other reference (directly or via an instance) to the class that gets deleted (an ivar, a temp that is alive during deletion (e.g. in a block), or if you stored in a dictionary: There is no way to "edit" these references on class delete.

The whole concept of "Obsolete class" is just that: "We deleted the class, but there are still users".

[Bajger]

I need to study more the code you are referring to. But problem starts in the moment, if some message is being sent to the obsolete instance, it shouldn't act normally. Now behavior and method lookup works in a same way as on normal instance (class). Maybe to move behavior (methods) to some "other" ObsoleteBehavior" instance, that would be related to obsoleteClass instance? Test case (or other code) that sends a message to the obsolete would initiate method lookup on obsolete, but original behavior is gone -> moved to other object (ObsoleteBehavior subclass). Test would then fail as expected.