and of course this is all my fault.  I should have checked and changed the test myself when I committed Tools-eem.788.  I apologise.

_,,,^..^,,,_ (phone)

> On May 5, 2018, at 1:14 PM, Eliot Miranda <eliot.miranda at gmail.com> wrote:
> 
> Hi David,
> 
>    sorry, I missed this thread.
> 
>> On May 1, 2018, at 6:13 PM, David T. Lewis <lewis at mail.msen.com> wrote:
>> 
>> That sounds right to me also.
>> 
>> @eliot - do you have a view on this? The fix would involve reverting one of your
>> changes from Tools-eem.788.
> 
> I like the name suggestions being as specific as is reading noble and I think anArray is likely a better suggestion than aSequence in most cases where an array is supplied, so my learning would be to change the test.
> 
>> 
>> Summary: The proposed fix for the DebuggerExtensionsTest>>#testCollectionsGeneralise
>> test failure would be:
>> 
>> 1) Change the test to say this:
>> 
>>   self assert: Collection name equals: Array new canonicalArgumentName.
>> 
>> instead of this:
>> 
>>   self assert: Collection name equals: Array new canonicalArgumentName.
>> 
>> 2) Delete the SequenceableCollection>>canonicalArgumentName that was added in Tools-eem.788
>> 
>> 
>> With those two changes, the test passes and the generated argument names
>> seem reasonable.
> 
> This is the tail wagging the dog.  The test should be changed, not the better suggestion discarded.  IMO.
> 
>> 
>> Dave
>> 
>> 
>> 
>>> On Tue, May 01, 2018 at 01:48:48AM +0000, Chris Cunningham wrote:
>>> I LIKE Array being anArray but the others being aCollection.   But that's
>>> just me.
>>> 
>>> aSequence sound weird to me
>>> 
>>> Cbc
>>> 
>>>>> On Mon, Apr 30, 2018, 18:03 David T. Lewis <lewis at mail.msen.com> wrote:
>>>>> 
>>>>>> On Mon, Apr 30, 2018 at 07:28:03PM -0400, David T. Lewis wrote:
>>>>>>> On Mon, Apr 30, 2018 at 02:13:15PM -0700, Frank Shearar wrote:
>>>>>>>> On 30 April 2018 at 13:23, David T. Lewis <lewis at mail.msen.com> wrote:
>>>>>>>> 
>>>>>>>> On Fri, Apr 27, 2018 at 10:36:17PM -0500, Tim Johnson wrote:
>>>>>>>> Hi,
>>>>>>>> 
>>>>>>>> I ran some tests and found a couple that are failing.  I also
>>>> checked
>>>>>>>> and found these tests don't fail in my 5.1 image.
>>>>>>>> 
>>>>>>>> BrowserTest>>#testBuildMessageCategoryBrowserEditString
>>>>>>>> DebuggerExtensionsTest>>#testCollectionsGeneralise
>>>>>>>> 
>>>>>>>> I think the first one might actually be a case where a bug was
>>>> fixed.
>>>>>>>> The test fails because of a timeout, because there is a dialog
>>>> on-screen
>>>>>>>> which is not returning.  Not sure, but the test may be intended to
>>>>>>>> declare that the dialog should appear, and now it is (?).
>>>>>>>> 
>>>>>>>> The second one, I don't understand.  There is no comment.
>>>>>>> 
>>>>>>> I don't understand it either, and strangely it has no method
>>>> timestamp.
>>>>>>> But the test was was introduced in April 2013 in this update:
>>>>>>> 
>>>>>>> Name: ToolsTests-fbs.62
>>>>>>> Author: fbs
>>>>>>> Time: 19 April 2013, 8:43:40.116 am
>>>>>>> UUID: 926d563e-d57b-44ec-b4e7-672010293c2b
>>>>>>> Ancestors: ToolsTests-nice.61
>>>>>>> 
>>>>>>> Tests for the new #canonicalArgumentName Debugger methods.
>>>>>>> 
>>>>>>> This is part of test coverage for #canonicalArgumentName, which is
>>>>>>> implemented
>>>>>>> in Object and also has no method timestamp or comment (!!!).
>>>>>>> 
>>>>>>> The canonicalArgumentName method is sent by many unit tests, so it
>>>> has very
>>>>>>> good coverage (even though I don't know the significance of this
>>>> failing
>>>>>>> test).
>>>>>>> 
>>>>>>> Aside from unit tests, it is sent by Message>>createStubMethod and
>>>> appears
>>>>>>> to be used when the debugger provides a template for implementing a
>>>> missing
>>>>>>> method, or for implementing a method override.
>>>>>>> 
>>>>>>> The test does this:
>>>>>>> 
>>>>>>> testCollectionsGeneralise
>>>>>>>     self assert: Collection name equals: Array new
>>>> canonicalArgumentName.
>>>>>>>     self assert: Collection name equals: OrderedCollection new
>>>>>>> canonicalArgumentName.
>>>>>>>     self assert: Collection name equals: LinkedList new
>>>>>>> canonicalArgumentName
>>>>>>> 
>>>>>>> 
>>>>>>> This looks like a regression that should be fixed such that the
>>>> tests pass
>>>>>>> again, and that also deserves a good method comment in
>>>>>>> Object>>cononicalArgumentName.
>>>>>>> 
>>>>>>> I think that some more background and explanation can probably be
>>>> found on
>>>>>>> squeak-dev circa April 2013, notably this reference:
>>>>>>> 
>>>>>>> http://lists.squeakfoundation.org/pipermail/squeak-dev/2013-
>>>>>>> April/170506.html
>>>>>>> 
>>>>>>> Which points to this:
>>>>>>> 
>>>>>>> Name: Tools-fbs.460
>>>>>>> Author: fbs
>>>>>>> Time: 19 April 2013, 8:40:24.143 am
>>>>>>> UUID: d5cf82c4-bda7-48ff-bfbd-e8b27d0a07d7
>>>>>>> Ancestors: Tools-fbs.459
>>>>>>> 
>>>>>>> When creating a stub method, give the argument names that
>>>> represent the
>>>>>>> (usual) desired name more accurately. For instance, Arrays,
>>>>>>> OrderedCollections
>>>>>>> and Sets all result in 'aCollection', ByteStrings and WideStrings
>>>> in
>>>>>>> 'aString',
>>>>>>> and so on.
>>>>>>> 
>>>>>>> So perhaps that last paragraph about "when creating a stub method..."
>>>>>>> might serve
>>>>>>> as a method comment for Object>>cononicalArgumentName?
>>>>>>> 
>>>>>>> Dave
>>>>>>> 
>>>>>> 
>>>>>> It was part of my work to improve the "JIT development" workflow, aka
>>>>>> "debugger driven development". See
>>>>>> 
>>>> http://lists.squeakfoundation.org/pipermail/squeak-dev/2013-April/170693.html
>>>>>> and http://bugs.squeak.org/view.php?id=7761
>>>>>> 
>>>>> 
>>>>> Thanks Frank,
>>>>> 
>>>>> And kudos for the test coverage, it is the sort of thing that almost
>>>>> certainly would have gone unnoticed otherwise.
>>>>> 
>>>>> Checking what has changed, the following two additions to the image
>>>> account for the test failure:
>>>>> 
>>>>> ArrayedCollection>>canonicalArgumentName  ==> 'Array'
>>>>> SequenceableCollection>>canonicalArgumentName ==> 'Sequence'
>>>>> 
>>>>> These entered the image here:
>>>>> 
>>>>> Name: Tools-eem.788
>>>>> Author: eem
>>>>> Time: 6 January 2018, 3:37:50.088654 pm
>>>>> UUID: bb90e476-4cf4-47bd-a8be-bc2785cc8504
>>>>> Ancestors: Tools-eem.787
>>>>> 
>>>>> Add some more canonicalArgumentName implementations for well-known
>>>> Collection subclasses.
>>>>> 
>>>>> So apparently the right thing to do is to update the unit tests to
>>>> reflect these additions.
>>>> 
>>>> At the risk of contradicting myself yet again,
>>>> 
>>>> According to testCollectionsGeneralise, we should have:
>>>> 
>>>> The argument prototype for an Array is 'aCollection'
>>>> The argument prototype for an OrderedCollection is 'aCollection'
>>>> The argument prototype for a LinkedList is 'aCollection'
>>>> 
>>>> However, in trunk we have this:
>>>> 
>>>> The argument prototype for an Array is 'anArray'
>>>> The argument prototype for an OrderedCollection is 'aSequence'
>>>> The argument prototype for a LinkedList is 'aSequence'
>>>> 
>>>> The original behavior as documented in the test seems better to me.
>>>> 
>>>> Opinions? Change the test, or revert the changes that lead to the test
>>>> failure?
>>>> 
>>>> Dave
>>>> 
>>>> 
>>>> 
>> 
>>> 
>> 
>>