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

David T. Lewis lewis at mail.msen.com
Tue May 1 01:03:48 UTC 2018


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