The Trunk: Tests-cmm.428.mcz
Locked
 
|
Chris Muller uploaded a new version of Tests to project The Trunk:
http://source.squeak.org/trunk/Tests-cmm.428.mcz ==================== Summary ==================== Name: Tests-cmm.428 Author: cmm Time: 13 March 2020, 6:09:37.220407 pm UUID: cd4af6d7-7fed-44c1-bfe7-64a2de3cb7ee Ancestors: Tests-nice.427 Tests for #caseError. =============== Diff against Tests-nice.427 =============== Item was added: + TestCase subclass: #CaseErrorTest + instanceVariableNames: '' + classVariableNames: '' + poolDictionaries: '' + category: 'Tests-Bugs'! Item was added: + ----- Method: CaseErrorTest>>printOn: (in category 'tests') ----- + printOn: aStream + 3 caseOf: + { [1] -> [aStream nextPutAll: 'option 1']. + [2] -> [aStream nextPutAll: 'option 2'] } + "caseError in printOn:"! Item was added: + ----- Method: CaseErrorTest>>testCaseError (in category 'tests') ----- + testCaseError + self + should: + [2 caseOf: + { [1] -> ['option 1'] }] + raise: Error! Item was added: + ----- Method: CaseErrorTest>>testCaseErrorInPrintOn (in category 'tests') ----- + testCaseErrorInPrintOn + self should: [ self printString ] raise: Error! |
Re: The Trunk: Tests-cmm.428.mcz
|
|
Hi Chris,
please have a look at Merge Request: #caseOf:otherwise: with arguments - I already tested #caseOf:[otherwise:] there in detail. If you think these tests are written badly, please let me know, I depend on your feedback :-) Von: Squeak-dev <[hidden email]> im Auftrag von [hidden email] <[hidden email]>
Gesendet: Samstag, 14. März 2020 00:09:39 An: [hidden email]; [hidden email] Betreff: [squeak-dev] The Trunk: Tests-cmm.428.mcz Chris Muller uploaded a new version of Tests to project The Trunk:
http://source.squeak.org/trunk/Tests-cmm.428.mcz ==================== Summary ==================== Name: Tests-cmm.428 Author: cmm Time: 13 March 2020, 6:09:37.220407 pm UUID: cd4af6d7-7fed-44c1-bfe7-64a2de3cb7ee Ancestors: Tests-nice.427 Tests for #caseError. =============== Diff against Tests-nice.427 =============== Item was added: + TestCase subclass: #CaseErrorTest + instanceVariableNames: '' + classVariableNames: '' + poolDictionaries: '' + category: 'Tests-Bugs'! Item was added: + ----- Method: CaseErrorTest>>printOn: (in category 'tests') ----- + printOn: aStream + 3 caseOf: + { [1] -> [aStream nextPutAll: 'option 1']. + [2] -> [aStream nextPutAll: 'option 2'] } + "caseError in printOn:"! Item was added: + ----- Method: CaseErrorTest>>testCaseError (in category 'tests') ----- + testCaseError + self + should: + [2 caseOf: + { [1] -> ['option 1'] }] + raise: Error! Item was added: + ----- Method: CaseErrorTest>>testCaseErrorInPrintOn (in category 'tests') ----- + testCaseErrorInPrintOn + self should: [ self printString ] raise: Error!
Carpe Squeak!
|
Re: The Trunk: Tests-cmm.428.mcz
|
|
Hi Christoph, This test is for Object>>#caseError only, not #caseOf:[otherwise:]. I started it out in ObjectTest, too, but then it looked like those tests were more "fundamental" while the purpose of this test was simply to expose the bug, so gave it its own class to expose the printing bug. I think it sounds fine to include your other tests and rename it more general (heh, would that be "TestCase subclass: #CaseTest" ? :) ). I took a quick look at your changeset and it looks good, the only question I would have is are those three Object new fixtures exercising the function in a different way? Matching up the 'a''s 'b''s and 'c's was a bit tedious, especially trying to figure out if they're testing something unique or the same function.. Regards, Chris On Mon, Mar 16, 2020 at 1:30 PM Thiede, Christoph <[hidden email]> wrote:
|
Re: The Trunk: Tests-cmm.428.mcz
|
|
Hi Chris,
> simply to expose the bug
Wait, now I see the actual bug you are talking about. It took me some time to find this out, and when this is fixed, no one (except you :)) will be able to understand what this test should do. We should definitively add some more explaining comments to this. Also, an ideal test should never hang up/time out but fail instead, should it? :)
In this case, I see we need an extra test class for it. (Well, personally I would've used #newSubclass, but I see this is a matter of taste ^^). The "regular" #caseOf: tests should go to ObjectTest.
> are those three Object new fixtures exercising the function in a different way
No, the only difference is the index of the matched item. If you think this is irrelevant we can drop the bs and the cs. :)
Best, Christoph Von: Squeak-dev <[hidden email]> im Auftrag von Chris Muller <[hidden email]>
Gesendet: Dienstag, 17. März 2020 00:09:18 An: The general-purpose Squeak developers list Cc: [hidden email] Betreff: Re: [squeak-dev] The Trunk: Tests-cmm.428.mcz Hi Christoph,
This test is for Object>>#caseError only, not #caseOf:[otherwise:]. I started it out in ObjectTest, too, but then it looked like those tests were more "fundamental" while the purpose of this test was simply to expose the bug, so gave it its own class
to expose the printing bug.
I think it sounds fine to include your other tests and rename it more general (heh, would that be "TestCase subclass: #CaseTest" ? :) ). I took a quick look at your changeset and it looks good, the only question I would have is are those three Object
new fixtures exercising the function in a different way? Matching up the 'a''s 'b''s and 'c's was a bit tedious, especially trying to figure out if they're testing something unique or the same function..
Regards,
Chris
On Mon, Mar 16, 2020 at 1:30 PM Thiede, Christoph <[hidden email]> wrote:
Carpe Squeak!
|
Re: The Trunk: Tests-cmm.428.mcz
|
|
I thought about that but then decided to simply leave the test generic, that #caseError is just working and not failing for ANY reason, not just the bug today. As you said, once it's fixed, any explanations here become ancient history that's of questionable usefulness to future readers.
Once it's fixed, it won't. :) Timeout was introduced because tests were hanging the CI server. I don't know any other way to make this test not hang as long as #caseError is broken.
Sure. Please feel free to move it to ObjectTest, include your tests, and improve it if you wish. I only care about getting #caseError reverted. General errors should be general and terse, not including the #printString's of, e.g., a 1000-element Array. Best, Chris |
Re: The Trunk: Tests-cmm.428.mcz
|
|
> I don't know any other way to make this test not hang as long as #caseError is broken.
Well, hypothetically, you could check thisContext stack :P
> General errors should be general and terse, not including the #printString's of, e.g., a 1000-element Array.
Absolutely.
I will try to merge this into ObjectTest as soon as I have an okay for the linked Merge Request :-)
Best,
Christoph
Von: Chris Muller <[hidden email]>
Gesendet: Dienstag, 17. März 2020 01:04:52 An: Thiede, Christoph Cc: The general-purpose Squeak developers list; [hidden email] Betreff: Re: [squeak-dev] The Trunk: Tests-cmm.428.mcz
I thought about that but then decided to simply leave the test generic, that #caseError is just working and not failing for ANY reason, not just the bug today. As you said, once it's fixed, any explanations here become ancient history that's of questionable
usefulness to future readers.
Once it's fixed, it won't. :) Timeout was introduced because tests were hanging the CI server. I don't know any other way to make this test not hang as long as #caseError is broken.
Sure. Please feel free to move it to ObjectTest, include your tests, and improve it if you wish. I only care about getting #caseError reverted. General errors should be general and terse, not including the #printString's of, e.g., a 1000-element Array.
Best,
Chris
Carpe Squeak!
|
|
In reply to this post by commits-2
This change causes "Cannot allocate memory" errors in the 32bit VM during testing (and therefore fails trunk builds): https://travis-ci.org/github/squeak-smalltalk/squeak-app/jobs/675293747#L6063 Please fix, Fabio Excerpt of stack trace: ........ 0xfe9d53a8 s [] in CaseErrorTest(Object)>printStringLimitedTo: 0xfe9d5430 s String class(SequenceableCollection class)>streamContents:limitedTo: 0xfe9d5538 s CaseErrorTest(Object)>printStringLimitedTo: 0xfe9d5598 s CaseErrorTest(Object)>printString 0xfe9d55f8 s CaseErrorTest(Object)>caseError 0xfe9d5658 s CaseErrorTest>printOn: 0xfe9d56b8 s [] in CaseErrorTest(Object)>printStringLimitedTo: 0xfe9d5740 s String class(SequenceableCollection class)>streamContents:limitedTo: 0xfe9d5848 s CaseErrorTest(Object)>printStringLimitedTo: 0xfe9d58a8 s CaseErrorTest(Object)>printString 0xfe9d5908 s CaseErrorTest(Object)>caseError 0xfe9d5968 s CaseErrorTest>printOn: ........ On Sat, Mar 14, 2020 at 12:09 AM <[hidden email]> wrote: Chris Muller uploaded a new version of Tests to project The Trunk: |
|
|
> On 16.04.2020, at 09:41, Fabio Niephaus <[hidden email]> wrote: > > This change causes "Cannot allocate memory" errors in the 32bit VM during testing (and therefore fails trunk builds): > https://travis-ci.org/github/squeak-smalltalk/squeak-app/jobs/675293747#L6063 > > Please fix, Well, it exposes a bug in trunk, right? This test case merely documents this :) -t > Fabio > > Excerpt of stack trace: > ........ > 0xfe9d53a8 s [] in CaseErrorTest(Object)>printStringLimitedTo: > 0xfe9d5430 s String class(SequenceableCollection class)>streamContents:limitedTo: > 0xfe9d5538 s CaseErrorTest(Object)>printStringLimitedTo: > 0xfe9d5598 s CaseErrorTest(Object)>printString > 0xfe9d55f8 s CaseErrorTest(Object)>caseError > 0xfe9d5658 s CaseErrorTest>printOn: > 0xfe9d56b8 s [] in CaseErrorTest(Object)>printStringLimitedTo: > 0xfe9d5740 s String class(SequenceableCollection class)>streamContents:limitedTo: > 0xfe9d5848 s CaseErrorTest(Object)>printStringLimitedTo: > 0xfe9d58a8 s CaseErrorTest(Object)>printString > 0xfe9d5908 s CaseErrorTest(Object)>caseError > 0xfe9d5968 s CaseErrorTest>printOn: > ........ > > On Sat, Mar 14, 2020 at 12:09 AM <[hidden email]> wrote: > Chris Muller uploaded a new version of Tests to project The Trunk: > http://source.squeak.org/trunk/Tests-cmm.428.mcz > > ==================== Summary ==================== > > Name: Tests-cmm.428 > Author: cmm > Time: 13 March 2020, 6:09:37.220407 pm > UUID: cd4af6d7-7fed-44c1-bfe7-64a2de3cb7ee > Ancestors: Tests-nice.427 > > Tests for #caseError. > > =============== Diff against Tests-nice.427 =============== > > Item was added: > + TestCase subclass: #CaseErrorTest > + instanceVariableNames: '' > + classVariableNames: '' > + poolDictionaries: '' > + category: 'Tests-Bugs'! > > Item was added: > + ----- Method: CaseErrorTest>>printOn: (in category 'tests') ----- > + printOn: aStream > + 3 caseOf: > + { [1] -> [aStream nextPutAll: 'option 1']. > + [2] -> [aStream nextPutAll: 'option 2'] } > + "caseError in printOn:"! > > Item was added: > + ----- Method: CaseErrorTest>>testCaseError (in category 'tests') ----- > + testCaseError > + self > + should: > + [2 caseOf: > + { [1] -> ['option 1'] }] > + raise: Error! > > Item was added: > + ----- Method: CaseErrorTest>>testCaseErrorInPrintOn (in category 'tests') ----- > + testCaseErrorInPrintOn > + self should: [ self printString ] raise: Error! > > > |
|
|
On Thu, Apr 16, 2020 at 9:46 AM Tobias Pape <[hidden email]> wrote:
If it is failing this hard, the test needs to be excluded from CI (e.g. via [1]). Our CI picks up new tests automatically, which is almost always what we want. Except when adding these kinds of tests (I'm going to exclude the test case now). This was merged a month ago, about the time trunk 64bit broke. I wish we'd take CI more seriously. Fabio
|
|
|
Trunk builds are working again: Bundles are at: Fabio On Thu, Apr 16, 2020 at 10:16 AM Fabio Niephaus <[hidden email]> wrote:
|
Re: The Trunk: Tests-cmm.428.mcz
|
Thanks Fabio! We'll take a look at this test. I think it is not a good idea to modify a test's #printString as part of a test because that #printString is used to show failing tests in tools. ;-) Best, Marcel
|
Re: The Trunk: Tests-cmm.428.mcz
|
|
The bigger issue is that we cannot interrupt recursive calls on printOn in "any case" .... :-/
|
Re: The Trunk: Tests-cmm.428.mcz
|
|
Why doesn't the test timeout catch it? I thought that was its primary purpose. On Fri, Apr 17, 2020 at 4:30 AM Marcel Taeumel <[hidden email]> wrote:
|
Re: The Trunk: Tests-cmm.428.mcz
|
|
> Why doesn't the test timeout catch it? Hmm... seems to be that there is an interference between recursive printOn: and Squeak's process scheduling. The VM is not even able to signal the UserInterruptSemaphore so CMD+Dot does not work either. Memory consumption is just going up and up and up ... Best, Marcel
|
|
|
> On 2020-04-20, at 8:05 AM, Marcel Taeumel <[hidden email]> wrote: > > > Why doesn't the test timeout catch it? > > Hmm... seems to be that there is an interference between recursive printOn: and Squeak's process scheduling. The VM is not even able to signal the UserInterruptSemaphore so CMD+Dot does not work either. Memory consumption is just going up and up and up ... Two possibilities occur to me 1) it's not actually running Smalltalk any more but is deep in the VM code trying to write out the stack. I've seen this quite a few times when recursion goes a bit feral. 2) Maybe it is time we reconnected the direct-semaphore that was once intended to whack the system upside the head when you hit the interrup key (which, we should remember, used to be configurable and not fixed as cmd/ctl-. ) tim -- tim Rowledge; [hidden email]; http://www.rowledge.org/tim Useful Latin Phrases:- Te precor dulcissime supplex! = Pretty please with a cherry on top! |
Re: The Trunk: Tests-cmm.428.mcz
|
|
> Maybe it is time we reconnected the direct-semaphore What do you mean? CMD+Comma? In Smalltalk-80 there was something like a second interrupt key to directly show the emergency evaluator... Best, Marcel
|
«
Return to Squeak - Dev
|
1 view|%1 views
| Free forum by Nabble | Edit this page |