[squeak-dev] Two tests failing in trunk but not 5.1

Eliot Miranda eliot.miranda at gmail.com
Sat May 5 20:14:28 UTC 2018


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
>>> 
>>> 
>>> 
> 
>> 
> 
> 


More information about the Squeak-dev mailing list