Two tests failing in trunk but not 5.1

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
12 messages Options
Reply | Threaded
Open this post in threaded view
|

Two tests failing in trunk but not 5.1

Tim Johnson-2
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

Reply | Threaded
Open this post in threaded view
|

Re: Two tests failing in trunk but not 5.1

David T. Lewis
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

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

Reply | Threaded
Open this post in threaded view
|

Re: Two tests failing in trunk but not 5.1

Frank Shearar-3
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

frank
 

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




Reply | Threaded
Open this post in threaded view
|

Re: Two tests failing in trunk but not 5.1

David T. Lewis
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.

Dave


Reply | Threaded
Open this post in threaded view
|

Re: Two tests failing in trunk but not 5.1

David T. Lewis
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


cbc
Reply | Threaded
Open this post in threaded view
|

Re: Two tests failing in trunk but not 5.1

cbc
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




Reply | Threaded
Open this post in threaded view
|

Re: Two tests failing in trunk but not 5.1

David T. Lewis
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
> >
> >
> >

>


cbc
Reply | Threaded
Open this post in threaded view
|

Re: Two tests failing in trunk but not 5.1

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

>





Reply | Threaded
Open this post in threaded view
|

Re: Two tests failing in trunk but not 5.1

David T. Lewis
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
> > > >
> > > >
> > > >
> >
> > >
> >
> >
> >

>


Reply | Threaded
Open this post in threaded view
|

Re: Two tests failing in trunk but not 5.1

David T. Lewis
I made the updates in trunk, and testCollectionsGeneralise passes again.

Dave


On Tue, May 01, 2018 at 11:56:34PM -0400, David T. Lewis wrote:

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

Reply | Threaded
Open this post in threaded view
|

Re: Two tests failing in trunk but not 5.1

Eliot Miranda-2
In reply to this post by David T. Lewis
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
>>>
>>>
>>>
>
>>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Two tests failing in trunk but not 5.1

Eliot Miranda-2
and of course this is all my fault.  I should have checked and changed the test myself when I committed Tools-eem.788.  I apologise.

_,,,^..^,,,_ (phone)

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