David/Elliot,

I think David means:

1) Change the test to say this:

        self assert: Array name equals: Array new canonicalArgumentName.

instead of this:

        self assert: Collection name equals: Array new
canonicalArgumentName.

Plus the rest.

On Tue, 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.
>
> 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.
>
> 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
> > >
> > >
> > >
>
> >
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20180501/4481b1b8/attachment-0001.html>