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