Mantis Bugtracker
  

Viewing Issue Simple Details Jump to Notes ] View Advanced ] Issue History ] Print ]
ID Category Severity Reproducibility Date Submitted Last Update
0007483 [Squeak] Janitorial major always 03-26-10 00:30 04-18-10 21:51
Reporter wiz View Status public  
Assigned To andreas
Priority high Resolution suspended  
Status closed   Product Version trunk
Summary 0007483: [tests] [fixes] Tests and fixes for unimplemented calls updated for sq 9787
Description For this one in a fresh squeak load the sunit tests for unimplemented calls.
Get a test runner.
Select the Unimpelmented Call Tests and run them.

One will pass and three will fail.

Load the fixes.

All but one test now passes.
Additional Information I have done and redone the fixes several times.

The tests are based on my workspace code and can be expanded to gradually challenge the fixing of most unimplemented calls.

I have put an high priority and urgency on it because of the oncoming 4.1 release.

These tests help catch unintended spelling errors and other gotcha's. And so serve as an excellent check on careless code commits.

The number of unimplemented calls in 3.10.2 was around 189.
The number reached a high point of 205 or so for the trunk. Nicolas was nice enough to bring them back down to 198.

The change set accompanying this report will bring them down to 189 again.




Attached Files  UnimplementedCallsTest.st [^] (5,089 bytes) 03-26-10 00:30
 FixSq9787Unimp-wiz.4.cs [^] (9,530 bytes) 03-26-10 00:31

- Relationships

SYSTEM WARNING: Creating default object from empty value

has duplicate 0007481closed  In sq 9721 many old unimplemented calls remain unfixed and new unimplemented calls have appeared as well. 
child of 0007480feedback  Issues that should be fixed in 4.1 

- Notes
(0013582 - 182 - 217 - 217 - 217 - 217 - 217)
wiz
03-26-10 00:33

UnimplementedCallsTest.st
this is the important one.


 FixSq9787Unimp-wiz.4.cs
these are a collection of patches to eliminate nine of the still remaining unimplemented calls.
 
(0013584 - 128 - 134 - 134 - 134 - 134 - 134)
nicolas cellier
03-26-10 20:35

This needs to be reviewed.
For example, you cannot replace primitiveFail (an injonction) with primitiveFailed (a constatation).
 
(0013586 - 346 - 394 - 394 - 394 - 394 - 394)
wiz
03-27-10 21:32

Hi Nicholas,

primitiveFail is AFAICT unimplemented because it is a misspelling of primitiveFailed.
If the code were ever called it would not understand #primitiveFail.

The tests are relevant even if you have concerns about my collection of fixes.
BTW You did a nice job of fixing half a dozen or so of these pests. thanx.

Cheers --Jer
 
(0013587 - 98 - 98 - 98 - 98 - 98 - 98)
wiz
03-27-10 21:33

also note that a lot of the fixes have been uploaded individually in the other (duplicate) report.
 
(0013588 - 363 - 363 - 363 - 363 - 363 - 363)
leves
03-27-10 22:25

There are methods, like #primitiveFail or #success: which are implemented by Interpreter. Since Interpreter is not part of the image anymore (it's in VMMaker), these methods are unimplemented in the image. But they can't be removed or replaced by #primitiveFailed because that would break VMMaker. Some uses of #primitiveFail are just typos, but others are valid.
 
(0013589 - 844 - 962 - 962 - 962 - 962 - 962)
wiz
03-27-10 22:40
edited on: 03-27-10 22:42

Ah. So which are these?

More interestingly. How should these unimplemented (in the image) calls be removed.

I suggest either removing the logic or adding a primitiveFail routine somewhere in the image.

Again the goal is to have tests that catch new and unintended unimplemented calls.

This will be easiest with fewer intend unimplemented calls allowed to stay in the image.

To paraphrase Red Skelton: "I just find them. I don't explain them."

Having all this source code available to one is not useful if the purpose of the pieces is undiscoverable. I often find that the limiting factor as I try to fix the curiostities I find in the image. Sometimes all I can do at first is make matters worse. That way at least someone pays attention.

Makes for an interesting time.

Yours in curiosity and service, --Jerome Peace

 
(0013590 - 1855 - 2205 - 2205 - 2205 - 2205 - 2205)
leves
03-27-10 23:14

I also think that it would be good if these methods could be moved to VMMaker, but that would mean code duplication, because these methods are used when the plugins are not available or when the primitive fails. This problem related to 7479.
I reviewed the changes of FixSq9787Unimp-wiz.4.cs and categorized them:

- translated primitives (should be kept as they are for now):
ADPCMCodec >> #privateEncodeStereo:
Bitmap >> #decompress:fromByteArray:at:
WarpBlt >> #warpBitsSmoothing:sourceMap:

- identical methods (nothing to do with them):
Behavior >> #allUndentMessages
ChangeSet >> #mailOut
PBBooleanPreferenceView >> #unload
PositionableStream >> #errorCantGoBack

- typo (should be fixed):
DisplayScreen >> #depth:width:height:fullscreen:

- new placeholder methods (I think these are bad, because the real implementors will replace them if they're loaded, making the packages dirty):
EToyVocabulary >> #applyMasterOrdering
Morph >> #gestureStart:

- methods sending methods implemented by external packages
UnixUnicodeInputInterpreter >> #initialize (we should probably keep it as is for now, but we should also check if keyValueIndex is right. Since this class has no users we should also consider removing it.)

- Preferences usage (the current methods use the fact that Preferences responds to #doesNotUnderstand:)
Object >> #doesNotUnderstand:

- Wrong fixes:
PseudoClass >> #theMetaClass (it should probably return "self metaClass", but clearly not "self class")
SequenceableCollection >> #assertSlopesWith:from:to: (moves an instance variable out of scope again, otherwise the methods are identical)


So IMHO we should fix:
DisplayScreen >> #depth:width:height:fullscreen:
PseudoClass >> #theMetaClass
and should consider changing:
Object >> #doesNotUnderstand:
UnixUnicodeInputInterpreter >> #initialize
 
(0013593 - 424 - 496 - 496 - 496 - 496 - 496)
wiz
03-29-10 03:19

leves,

Thank you for reviewing my fix attempts.

As a one man shop its really hard to make the right guesses the first time out.

The important thing is to have the tests in the image to prevent new unintended occurances.

The level at which the tests cut off can be adjusted.

Also which image did you use to examine the code? It sounds like some of the fixes have been corrected independently.

Cheers --Jer
 
(0013607 - 450 - 540 - 1304 - 1304 - 1304 - 1304)
nicolas cellier
03-30-10 21:29

#allUnSentMessagesIn: fix:
http://source.squeak.org/trunk/System-nice.299.mcz [^]

#errorCantGoBack fix:
http://source.squeak.org/trunk/Collections-nice.348.mcz [^]

mailout Preferences checkForSlip fix:
http://source.squeak.org/trunk/System-nice.300.mcz [^]

DisplayScreen fix:
http://source.squeak.org/trunk/Graphics-nice.129.mcz [^]

PseudoClass fix:
http://source.squeak.org/trunk/System-nice.305.mcz [^]

Others won't fix (until we find a better fix)
 
(0013635 - 292 - 304 - 304 - 304 - 304 - 304)
andreas
04-08-10 03:38

I am suspending this for the 4.1 release. We shouldn't push this further at this point since these are not regressions and we'll have to think about this before making further progress.

Please reopen after 4.1 goes out or create a new issues for anything that wasn't handled in this round.
 

- Issue History
Date Modified Username Field Change
03-26-10 00:30 wiz New Issue
03-26-10 00:30 wiz File Added: UnimplementedCallsTest.st
03-26-10 00:31 wiz File Added: FixSq9787Unimp-wiz.4.cs
03-26-10 00:33 wiz Note Added: 0013582
03-26-10 00:35 wiz Relationship added has duplicate 0007481
03-26-10 00:35 wiz Relationship added child of 0007480
03-26-10 20:35 nicolas cellier Note Added: 0013584
03-27-10 21:32 wiz Note Added: 0013586
03-27-10 21:33 wiz Note Added: 0013587
03-27-10 22:25 leves Note Added: 0013588
03-27-10 22:40 wiz Note Added: 0013589
03-27-10 22:42 wiz Note Edited: 0013589
03-27-10 23:14 leves Note Added: 0013590
03-29-10 03:19 wiz Note Added: 0013593
03-30-10 21:29 nicolas cellier Note Added: 0013607
03-30-10 21:29 nicolas cellier Status new => pending
04-08-10 03:38 andreas Status pending => resolved
04-08-10 03:38 andreas Resolution open => suspended
04-08-10 03:38 andreas Assigned To  => andreas
04-08-10 03:38 andreas Note Added: 0013635
04-18-10 21:51 andreas Status resolved => closed


Mantis 1.0.8[^]
Copyright © 2000 - 2007 Mantis Group
92 total queries executed.
55 unique queries executed.
Powered by Mantis Bugtracker