Issue 3581 in pharo: [Failing test] 1.1.1 Full: MethodWrappers.Tests.ObjectAsOneTimeMethodWrapperTest.testInstallOnClassCategory

Status: Accepted
Owner: [hidden email]
Labels: Milestone-1.1.2

New issue 3581 by [hidden email]: [Failing test] 1.1.1 Full:  
MethodWrappers.Tests.ObjectAsOneTimeMethodWrapperTest.testInstallOnClassCategory
http://code.google.com/p/pharo/issues/detail?id=3581

   
MethodWrappers.Tests.ObjectAsOneTimeMethodWrapperTest.testInstallOnClassCategory

fails sometimes.

Updates:
        Labels: Milestone-1.2-DevImage

Comment #1 on issue 3581 by [hidden email]: [Failing test] 1.1.1  
Full:  
MethodWrappers.Tests.ObjectAsOneTimeMethodWrapperTest.testInstallOnClassCategory
http://code.google.com/p/pharo/issues/detail?id=3581

(No comment was entered for this change.)

Updates:
        Labels: -Milestone-1.1.2 Milestone-1.1.2-DevImage

Comment #2 on issue 3581 by [hidden email]: [Failing test] 1.1.1  
Full:  
MethodWrappers.Tests.ObjectAsOneTimeMethodWrapperTest.testInstallOnClassCategory
http://code.google.com/p/pharo/issues/detail?id=3581

(No comment was entered for this change.)

Updates:
        Cc: alexandre.bergel

Comment #3 on issue 3581 by [hidden email]: [Failing test] 1.1.1  
Full:  
MethodWrappers.Tests.ObjectAsOneTimeMethodWrapperTest.testInstallOnClassCategory
http://code.google.com/p/pharo/issues/detail?id=3581

What do we do? Remove the test or the whole package?

Comment #4 on issue 3581 by [hidden email]: [Failing test] 1.1.1  
Full:  
MethodWrappers.Tests.ObjectAsOneTimeMethodWrapperTest.testInstallOnClassCategory
http://code.google.com/p/pharo/issues/detail?id=3581

We should remove this package from 1.2

Comment #5 on issue 3581 by [hidden email]: [Failing test] 1.1.1  
Full:  
MethodWrappers.Tests.ObjectAsOneTimeMethodWrapperTest.testInstallOnClassCategory
http://code.google.com/p/pharo/issues/detail?id=3581

I will have a look.
now if we cannot reproduce it on the server this is really strange.
We can move it as expected failures

Updates:
        Labels: -Milestone-1.2-DevImage

Comment #6 on issue 3581 by [hidden email]: [Failing test] 1.1.1  
Full:  
MethodWrappers.Tests.ObjectAsOneTimeMethodWrapperTest.testInstallOnClassCategory
http://code.google.com/p/pharo/issues/detail?id=3581

Not failing in 1.2

Comment #7 on issue 3581 by [hidden email]: [Failing test] 1.1.1 Full:  
MethodWrappers.Tests.ObjectAsOneTimeMethodWrapperTest.testInstallOnClassCategory
http://code.google.com/p/pharo/issues/detail?id=3581

This looks like an issue with the method cache.

ObjectAsMethodWrapper>>install manipulates the methodDict directly.

We should change the following code in that method from
self wrappedClass methodDict at: self selector put: self.
to rather be
self wrappedClass addSelector: self selector withMethod: self.

This ensures that the method cache is flushed in  
Behaviour>>basicAddSelector:withMethod:

See CompiledMethod>>flushCache {accessing} and Symbol>>flushCache {system  
primitives}

Comment #8 on issue 3581 by marianopeck: [Failing test] 1.1.1 Full:  
MethodWrappers.Tests.ObjectAsOneTimeMethodWrapperTest.testInstallOnClassCategory
http://code.google.com/p/pharo/issues/detail?id=3581

mmmm I don't understand. Check CompiledMethod >> at:put:

at: key put: value
        "Set the value at key to be value."
        | index |
        index := self findElementOrNil: key.
        (self basicAt: index)
                ifNil:
                        [tally := tally + 1.
                        self basicAt: index put: key]
                ifNotNil:
                        [(array at: index) flushCache].
        array at: index put: value.
        self fullCheck.
        ^ value



I think the problem is because ObjectAsMethodWrapper >> flushCache is empty.
what do you think?

Comment #9 on issue 3581 by [hidden email]: [Failing test] 1.1.1 Full:  
MethodWrappers.Tests.ObjectAsOneTimeMethodWrapperTest.testInstallOnClassCategory
http://code.google.com/p/pharo/issues/detail?id=3581

It will probably work if we modify ObjectAsMethodWrapper>>flushCache
        self selector flushCache.

Would it not be safer to call the addSelector:withMethod as it sets up any  
other state. i.e. Behavior>>addSelectorSilently:withMethod: is called which  
calls Behaviour>>registerLocalSelector: which I think traits use.

Updates:
        Labels: -Milestone-1.1.2-DevImage Milestone-1.2-DevImage

Comment #10 on issue 3581 by [hidden email]: [Failing test] 1.1.1  
Full:  
MethodWrappers.Tests.ObjectAsOneTimeMethodWrapperTest.testInstallOnClassCategory
http://code.google.com/p/pharo/issues/detail?id=3581

Problem in 1.2 (1.1 we don't care)

Comment #11 on issue 3581 by marianopeck: [Failing test] 1.1.1 Full:  
MethodWrappers.Tests.ObjectAsOneTimeMethodWrapperTest.testInstallOnClassCategory
http://code.google.com/p/pharo/issues/detail?id=3581

The problem is that if you use addSelector:withMethod  then you will need  
to implement a lot of messages, like #methodClass: , #selector: , #pragmas,  
etc.
In summary, a lot of methods from CompiledMethod. Notice that  
ObjectAsMethodWrapper is subclass of Object, not CompiledMethod.

Cheers

Mariano

Comment #12 on issue 3581 by [hidden email]: [Failing test] 1.1.1 Full:  
MethodWrappers.Tests.ObjectAsOneTimeMethodWrapperTest.testInstallOnClassCategory
http://code.google.com/p/pharo/issues/detail?id=3581

Ok I see now.

Then is it safe to just do
ObjectAsMethodWrapper>>flushCache
        self selector flushCache.

Updates:
        Status: Closed

Comment #13 on issue 3581 by marianopeck: [Failing test] 1.1.1 Full:  
MethodWrappers.Tests.ObjectAsOneTimeMethodWrapperTest.testInstallOnClassCategory
http://code.google.com/p/pharo/issues/detail?id=3581

Yes, that's perfect. THe other possibility was to implemented this way_

ObjectAsMethodWrapper>>flushCache
        <primitive: 116>

Anyway, I commit your version because in this case it is cleaner.


Name: MethodWrappers-MarianoMartinezPeck.6
Author: MarianoMartinezPeck
Time: 16 March 2011, 10:57:08 am
UUID: 48366448-4da1-4351-98f5-8a5f16d52be2
Ancestors: MethodWrappers-MarianoMartinezPeck.5

Implement ObjectAsMethodWrapper>>flushCache so that it flush the entries in  
the cache that match the selector