Two tests failing in trunk but not 5.1

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

>
> Image/VM info:
>
> Squeak6.0alpha
> latest update: #17922
> Image format 6521 (32 bit)
>
> Virtual Machine
> ---------------
> C:\Users\tcj\Desktop\squeak.cog.spur_win32x86_201803080952\Squeak.exe
> Croquet Closure Cog[Spur] VM [CoInterpreterPrimitives
> VMMaker.oscog-eem.2347]
> Win32 built on Mar  8 2018 11:08:39 GMT Compiler: 6.4.0
> platform sources revision VM: 201803080952
>
> Cheers,
> Tim
>

> On 30 April 2018 at 13:23, David T. Lewis <[hidden email]> 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
>

> On Mon, Apr 30, 2018 at 02:13:15PM -0700, Frank Shearar wrote:
> > On 30 April 2018 at 13:23, David T. Lewis <[hidden email]> 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.

> 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 <[hidden email]> 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 <[hidden email]> 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
> >
> >
> >

> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]>
> > 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
> > > >
> > > >
> > > >
> >
> > >
> >
> >
> >

> Sorry, yes that is what I meant to say.
>
> Dave
>
>
> On Tue, May 01, 2018 at 08:04:41PM -0700, Chris Cunningham wrote:
> > 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]>
> > > 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
> > > > >
> > > > >
> > > > >
> > >
> > > >
> > >
> > >
> > >
>
> >
>
>

>
> 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 <[hidden email]> 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 <[hidden email]> 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
>>>
>>>
>>>
>
>>
>
>

> On May 5, 2018, at 1:14 PM, Eliot Miranda <[hidden email]> wrote:
>
> Hi David,
>
>    sorry, I missed this thread.
>
>> On May 1, 2018, at 6:13 PM, David T. Lewis <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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
>>>>
>>>>
>>>>
>>
>>>
>>
>>