[Pharo-project] 20381 running the test does not work ...
There is a bug blocking the image. SubscriptOutOfBounds: 0 in the SystemProgressMorph class updateJob:
Apparently there was failing test which invoked a on:fork:â¦. This is strange since the processTest were red. Now running them alone brings green tests. Stef On Nov 3, 2012, at 12:53 PM, Stéphane Ducasse wrote:
There is a bug blocking the image. SubscriptOutOfBounds: 0
in the SystemProgressMorph class updateJob:
Executing this test method leads to an error MCStReaderTest debug: #testCommentWithStyle SmallInteger>>/ Job>>progress JobChange>>progress SystemProgressMorph class>>updateJob: MessageSend>>value: MessageSend>>cull: MessageSend>>cull:cull: AnnouncementSubscription>>deliver: in Block: [action cull: anAnnouncement cull: announcer] BlockClosure>>on:do: BlockClosure>>on:fork: in Block: [Processor terminateActive] testCommentWithStyle "self debug: #testCommentWithStyle" | reader | reader := MCStReader on: self commentWithStyle readStream. reader definitions But the error is raised by the progress bar Job>>progress ^ (currentValue - min) / (max - min) Raises a ZeroDivide Because max and min are equal to 1. What is really strange is that if I try with another test - green it will run perfectly - if I introduce a bug in another green test it will run and lead to the debugger and not the error SmallInteger>>/ Job>>progress JobChange>>progress SystemProgressMorph class>>updateJob: MessageSend>>value: MessageSend>>cull: MessageSend>>cull:cull: AnnouncementSubscription>>deliver: in Block: [action cull: anAnnouncement cull: announcer] BlockClosure>>on:do: BlockClosure>>on:fork: in Block: [Processor terminateActive]
I'm wondering if the progress bar code is not bogus and that it is created with wrong values.
Apparently there was failing test which invoked a on:fork:â¦.
This is strange since the processTest were red. Now running them alone brings green tests.
<Screen Shot 2012-11-03 at 7.42.47 PM.pdf> Stef On Nov 3, 2012, at 12:53 PM, Stéphane Ducasse wrote:
There is a bug blocking the image. SubscriptOutOfBounds: 0
in the SystemProgressMorph class updateJob:
Yes apparently the code run by the test uses the progress bar (do display progress) and at some point put its max to 1 (since there is only one definition to process) => boum. since in that case we get a zeroDivide. Stef On Nov 3, 2012, at 9:51 PM, Stéphane Ducasse wrote:
I'm wondering if the progress bar code is not bogus and that it is created with wrong values.
Apparently there was failing test which invoked a on:fork:â¦.
This is strange since the processTest were red. Now running them alone brings green tests.
<Screen Shot 2012-11-03 at 7.42.47 PM.pdf> Stef On Nov 3, 2012, at 12:53 PM, Stéphane Ducasse wrote:
There is a bug blocking the image. SubscriptOutOfBounds: 0
in the SystemProgressMorph class updateJob:
Hi Stef, I have seen this problem too, and I think that JobTest is guilty for this behaviour, since its tearDown method removes all bars, even though they are not his to remove. Try removing the tearDown... it work in my case JobTest>>tearDown SystemProgressMorph uniqueInstance bars do: [ :e | e close ]. I hope that this helps... If not I think that there is a problem with the SystemProgressMorph api in a concurrent context... An arbitrary class should not be able to remove bars from our nice progress morph... Cheers, Ciprian On Sat, Nov 3, 2012 at 9:57 PM, Stéphane Ducasse <stephane.ducasse@inria.fr>wrote:
Yes apparently the code run by the test uses the progress bar (do display progress) and at some point put its max to 1 (since there is only one definition to process) => boum.
since in that case we get a zeroDivide.
Stef
On Nov 3, 2012, at 9:51 PM, Stéphane Ducasse wrote:
I'm wondering if the progress bar code is not bogus and that it is created with wrong values.
Apparently there was failing test which invoked a on:fork:â¦.
This is strange since the processTest were red. Now running them alone brings green tests.
<Screen Shot 2012-11-03 at 7.42.47 PM.pdf> Stef On Nov 3, 2012, at 12:53 PM, Stéphane Ducasse wrote:
There is a bug blocking the image. SubscriptOutOfBounds: 0
in the SystemProgressMorph class updateJob:
-- Dr. Ciprian TEODOROV Ingénieur Développement CAO tél : 06 08 54 73 48 mail : ciprian.teodorov@gmail.com www.teodorov.ro
Ouch two problems! I'm also wondering why we have SystemProgressItemMorph and ProgressBarMorph in addition to SystemProgressMorph and of course JobProgressMorph and JobProgressBarMorph to me it looks like this is ok to have - one with the forbidden flag = JobProgressMorph - a default one = SystemProgressMorph and the bar = ProgressBarMorph and I saw that the size of the SystemProgressMorph is not passed to the its inner elements leading to this ugly bar getting out of the size of the progress bar. Stef On Nov 3, 2012, at 10:12 PM, Ciprian Teodorov wrote:
Hi Stef,
I have seen this problem too, and I think that JobTest is guilty for this behaviour, since its tearDown method removes all bars, even though they are not his to remove. Try removing the tearDown... it work in my case
JobTest>>tearDown SystemProgressMorph uniqueInstance bars do: [ :e | e close ].
I hope that this helps...
If not I think that there is a problem with the SystemProgressMorph api in a concurrent context... An arbitrary class should not be able to remove bars from our nice progress morph...
Cheers, Ciprian
On Sat, Nov 3, 2012 at 9:57 PM, Stéphane Ducasse <stephane.ducasse@inria.fr> wrote: Yes apparently the code run by the test uses the progress bar (do display progress) and at some point put its max to 1 (since there is only one definition to process) => boum.
since in that case we get a zeroDivide.
Stef
On Nov 3, 2012, at 9:51 PM, Stéphane Ducasse wrote:
I'm wondering if the progress bar code is not bogus and that it is created with wrong values.
Apparently there was failing test which invoked a on:fork:â¦.
This is strange since the processTest were red. Now running them alone brings green tests.
<Screen Shot 2012-11-03 at 7.42.47 PM.pdf> Stef On Nov 3, 2012, at 12:53 PM, Stéphane Ducasse wrote:
There is a bug blocking the image. SubscriptOutOfBounds: 0
in the SystemProgressMorph class updateJob:
-- Dr. Ciprian TEODOROV Ingénieur Développement CAO
tél : 06 08 54 73 48 mail : ciprian.teodorov@gmail.com www.teodorov.ro
While riding under the rain I was thinking and I guess that SystemProgressMorph => singleton that accepts several tasks and bars ProgressBarMorph => basic elements Job: progress bar + title + I will probably repackage this together and add comments then I will try to fix the size problem. Stef On Nov 3, 2012, at 10:48 PM, Stéphane Ducasse wrote:
Ouch two problems!
I'm also wondering why we have
SystemProgressItemMorph and ProgressBarMorph in addition to SystemProgressMorph
and of course JobProgressMorph and JobProgressBarMorph
to me it looks like this is ok to have
- one with the forbidden flag = JobProgressMorph - a default one = SystemProgressMorph and the bar = ProgressBarMorph
and I saw that the size of the SystemProgressMorph is not passed to the its inner elements leading to this ugly bar getting out of the size of the progress bar.
Stef
On Nov 3, 2012, at 10:12 PM, Ciprian Teodorov wrote:
Hi Stef,
I have seen this problem too, and I think that JobTest is guilty for this behaviour, since its tearDown method removes all bars, even though they are not his to remove. Try removing the tearDown... it work in my case
JobTest>>tearDown SystemProgressMorph uniqueInstance bars do: [ :e | e close ].
I hope that this helps...
If not I think that there is a problem with the SystemProgressMorph api in a concurrent context... An arbitrary class should not be able to remove bars from our nice progress morph...
Cheers, Ciprian
On Sat, Nov 3, 2012 at 9:57 PM, Stéphane Ducasse <stephane.ducasse@inria.fr> wrote: Yes apparently the code run by the test uses the progress bar (do display progress) and at some point put its max to 1 (since there is only one definition to process) => boum.
since in that case we get a zeroDivide.
Stef
On Nov 3, 2012, at 9:51 PM, Stéphane Ducasse wrote:
I'm wondering if the progress bar code is not bogus and that it is created with wrong values.
Apparently there was failing test which invoked a on:fork:â¦.
This is strange since the processTest were red. Now running them alone brings green tests.
<Screen Shot 2012-11-03 at 7.42.47 PM.pdf> Stef On Nov 3, 2012, at 12:53 PM, Stéphane Ducasse wrote:
There is a bug blocking the image. SubscriptOutOfBounds: 0
in the SystemProgressMorph class updateJob:
-- Dr. Ciprian TEODOROV Ingénieur Développement CAO
tél : 06 08 54 73 48 mail : ciprian.teodorov@gmail.com www.teodorov.ro
There are a couple of things missing in the progress refactoring I started with sean - Job/ProgressBar should only use a single progress variable (0.0 - 1.0), no more unbounded min/max/current which leads to duplicate size calculations everywhere - The Visual part should be adapted to only take a single variable into account - The ProgressMorph only updates maximally every 50ms no matter how many change/updates it got - Make sure that concurrent progress are supported (currently only the last added progress morph is updated) On 2012-11-03, at 22:48, Stéphane Ducasse <stephane.ducasse@inria.fr> wrote:
Ouch two problems!
I'm also wondering why we have
SystemProgressItemMorph and ProgressBarMorph in addition to SystemProgressMorph
and of course JobProgressMorph and JobProgressBarMorph
to me it looks like this is ok to have
- one with the forbidden flag = JobProgressMorph - a default one = SystemProgressMorph and the bar = ProgressBarMorph
definitely the hierarchy is mess there. And there are indeed only 4 items needed - the Global displayed SystemProgressMorph which contains multiple progress indicators and the logics to force update the UI even when stuff runs in the UI thread - the item (title + progress bar) - the progress bar - the job item (title + progress bar + interrupt button)
and I saw that the size of the SystemProgressMorph is not passed to the its inner elements leading to this ugly bar getting out of the size of the progress bar.
the size should and has to be adapted automatically. Currently it's not properly updated since I removed a forced refresh, which makes it >factor 10 faster. Of course this should be rewritten once the previous points have been fixed. My idea was that the progress bar only grows but newer shrinks so that you avoid this nasty size flickering when lots of subjobs are launched.
Stef
On Nov 3, 2012, at 10:12 PM, Ciprian Teodorov wrote:
Hi Stef,
I have seen this problem too, and I think that JobTest is guilty for this behaviour, since its tearDown method removes all bars, even though they are not his to remove. Try removing the tearDown... it work in my case
JobTest>>tearDown SystemProgressMorph uniqueInstance bars do: [ :e | e close ].
I hope that this helps...
If not I think that there is a problem with the SystemProgressMorph api in a concurrent context... An arbitrary class should not be able to remove bars from our nice progress morph...
Cheers, Ciprian
On Sat, Nov 3, 2012 at 9:57 PM, Stéphane Ducasse <stephane.ducasse@inria.fr> wrote: Yes apparently the code run by the test uses the progress bar (do display progress) and at some point put its max to 1 (since there is only one definition to process) => boum.
since in that case we get a zeroDivide.
Stef
On Nov 3, 2012, at 9:51 PM, Stéphane Ducasse wrote:
I'm wondering if the progress bar code is not bogus and that it is created with wrong values.
Apparently there was failing test which invoked a on:fork:â¦.
This is strange since the processTest were red. Now running them alone brings green tests.
<Screen Shot 2012-11-03 at 7.42.47 PM.pdf> Stef On Nov 3, 2012, at 12:53 PM, Stéphane Ducasse wrote:
There is a bug blocking the image. SubscriptOutOfBounds: 0
in the SystemProgressMorph class updateJob:
-- Dr. Ciprian TEODOROV Ingénieur Développement CAO
tél : 06 08 54 73 48 mail : ciprian.teodorov@gmail.com www.teodorov.ro
Thanks I will try to digest that.
- Job/ProgressBar should only use a single progress variable (0.0 - 1.0), no more unbounded min/max/current which leads to duplicate size calculations everywhere - The Visual part should be adapted to only take a single variable into account - The ProgressMorph only updates maximally every 50ms no matter how many change/updates it got - Make sure that concurrent progress are supported (currently only the last added progress morph is updated)
On 2012-11-03, at 22:48, Stéphane Ducasse <stephane.ducasse@inria.fr> wrote:
Ouch two problems!
I'm also wondering why we have
SystemProgressItemMorph and ProgressBarMorph in addition to SystemProgressMorph
and of course JobProgressMorph and JobProgressBarMorph
to me it looks like this is ok to have
- one with the forbidden flag = JobProgressMorph - a default one = SystemProgressMorph and the bar = ProgressBarMorph
definitely the hierarchy is mess there. And there are indeed only 4 items needed - the Global displayed SystemProgressMorph which contains multiple progress indicators and the logics to force update the UI even when stuff runs in the UI thread - the item (title + progress bar) - the progress bar - the job item (title + progress bar + interrupt button)
and I saw that the size of the SystemProgressMorph is not passed to the its inner elements leading to this ugly bar getting out of the size of the progress bar.
the size should and has to be adapted automatically. Currently it's not properly updated since I removed a forced refresh, which makes it >factor 10 faster. Of course this should be rewritten once the previous points have been fixed.
My idea was that the progress bar only grows but newer shrinks so that you avoid this nasty size flickering when lots of subjobs are launched.
Stef
On Nov 3, 2012, at 10:12 PM, Ciprian Teodorov wrote:
Hi Stef,
I have seen this problem too, and I think that JobTest is guilty for this behaviour, since its tearDown method removes all bars, even though they are not his to remove. Try removing the tearDown... it work in my case
JobTest>>tearDown SystemProgressMorph uniqueInstance bars do: [ :e | e close ].
I hope that this helps...
If not I think that there is a problem with the SystemProgressMorph api in a concurrent context... An arbitrary class should not be able to remove bars from our nice progress morph...
Cheers, Ciprian
On Sat, Nov 3, 2012 at 9:57 PM, Stéphane Ducasse <stephane.ducasse@inria.fr> wrote: Yes apparently the code run by the test uses the progress bar (do display progress) and at some point put its max to 1 (since there is only one definition to process) => boum.
since in that case we get a zeroDivide.
Stef
On Nov 3, 2012, at 9:51 PM, Stéphane Ducasse wrote:
I'm wondering if the progress bar code is not bogus and that it is created with wrong values.
Apparently there was failing test which invoked a on:fork:â¦.
This is strange since the processTest were red. Now running them alone brings green tests.
<Screen Shot 2012-11-03 at 7.42.47 PM.pdf> Stef On Nov 3, 2012, at 12:53 PM, Stéphane Ducasse wrote:
There is a bug blocking the image. SubscriptOutOfBounds: 0
in the SystemProgressMorph class updateJob:
-- Dr. Ciprian TEODOROV Ingénieur Développement CAO
tél : 06 08 54 73 48 mail : ciprian.teodorov@gmail.com www.teodorov.ro
do you know what is the ProgressIndicatorMorph and LoggingSystemProgressItemMorph (which inherits from Object)? I will start to group all the classes into a separate package. Potentially rename them so that we can edit them and try to fix that. Stef On Nov 4, 2012, at 11:30 AM, Camillo Bruni wrote:
definitely the hierarchy is mess there. And there are indeed only 4 items needed - the Global displayed SystemProgressMorph which contains multiple progress indicators and the logics to force update the UI even when stuff runs in the UI thread - the item (title + progress bar) - the progress bar - the job item (title + progress bar + interrupt button)
camillo one of the problem that we have is that the refactoring stopped in the middle because displayProgressFrom: minVal to: maxVal during: workBlock now works with job and this is nice but job requires but all the user of displayProgressFrom: minVal to: maxVal during: workBlock are using bar current: aNumber and not a value between 0 and 1.0. So this means that we will a lot of broken code. Am I correct? Stef displayProgressFrom: minVal to: maxVal during: workBlock "Display this string as a caption over a progress bar while workBlock is evaluated. 'Now here''s some Real Progress' displayProgressFrom: 0 to: 10 during: [:bar | 1 to: 10 do: [:x | bar value: x. (Delay forMilliseconds: 500) wait]]. " ^ UIManager default displayProgress: self from: minVal to: maxVal during: workBlock displayProgress: titleString from: minVal to: maxVal during: workBlock "SystemProgressMorph show: titleString from: minVal to: during: " ^ workBlock asJob title: titleString; min: minVal; max: maxVal; run.
On 2012-11-30, at 18:15, stephane ducasse <stephane.ducasse@free.fr> wrote:
camillo
one of the problem that we have is that the refactoring stopped in the middle because displayProgressFrom: minVal to: maxVal during: workBlock now works with job and this is nice but job requires but all the user of displayProgressFrom: minVal to: maxVal during: workBlock are using bar current: aNumber and not a value between 0 and 1.0.
So this means that we will a lot of broken code. Am I correct?
right now Job works still with min/max/current so I still have to do the full transition. Ideally no code will be broken, since most people use progress bars via one of the public interfaces, either on String, Collections or directly on the UI Manager. => these will be the main places to change it. I think I can have a look at it today or tomorrow and finish the refactoring.
I think that the definition of progress is wrong since I was not running the JobTest. But in addition I think that you are right, close the bars looks to me a bad idea. Stef On Nov 3, 2012, at 10:12 PM, Ciprian Teodorov wrote:
Hi Stef,
I have seen this problem too, and I think that JobTest is guilty for this behaviour, since its tearDown method removes all bars, even though they are not his to remove. Try removing the tearDown... it work in my case
JobTest>>tearDown SystemProgressMorph uniqueInstance bars do: [ :e | e close ].
I hope that this helps...
If not I think that there is a problem with the SystemProgressMorph api in a concurrent context... An arbitrary class should not be able to remove bars from our nice progress morph...
Cheers, Ciprian
On Sat, Nov 3, 2012 at 9:57 PM, Stéphane Ducasse <stephane.ducasse@inria.fr> wrote: Yes apparently the code run by the test uses the progress bar (do display progress) and at some point put its max to 1 (since there is only one definition to process) => boum.
since in that case we get a zeroDivide.
Stef
On Nov 3, 2012, at 9:51 PM, Stéphane Ducasse wrote:
I'm wondering if the progress bar code is not bogus and that it is created with wrong values.
Apparently there was failing test which invoked a on:fork:â¦.
This is strange since the processTest were red. Now running them alone brings green tests.
<Screen Shot 2012-11-03 at 7.42.47 PM.pdf> Stef On Nov 3, 2012, at 12:53 PM, Stéphane Ducasse wrote:
There is a bug blocking the image. SubscriptOutOfBounds: 0
in the SystemProgressMorph class updateJob:
-- Dr. Ciprian TEODOROV Ingénieur Développement CAO
tél : 06 08 54 73 48 mail : ciprian.teodorov@gmail.com www.teodorov.ro
SystemProgressMorph show: 'Doing...' from: 0 to: 1 during: [ :bar | 1 to: 2 do: [ :x | bar current: x. (Delay forMilliseconds: 20) wait "Just to slow it down so we can see what's going on" ] ]. Works while SystemProgressMorph show: 'Doing...' from: 1 to: 1 during: [ :bar | 1 to: 2 do: [ :x | bar current: x. (Delay forMilliseconds: 20) wait "Just to slow it down so we can see what's going on" ] ]. not :) Stef On Nov 3, 2012, at 10:12 PM, Ciprian Teodorov wrote:
Hi Stef,
I have seen this problem too, and I think that JobTest is guilty for this behaviour, since its tearDown method removes all bars, even though they are not his to remove. Try removing the tearDown... it work in my case
JobTest>>tearDown SystemProgressMorph uniqueInstance bars do: [ :e | e close ].
I hope that this helps...
If not I think that there is a problem with the SystemProgressMorph api in a concurrent context... An arbitrary class should not be able to remove bars from our nice progress morph...
Cheers, Ciprian
On Sat, Nov 3, 2012 at 9:57 PM, Stéphane Ducasse <stephane.ducasse@inria.fr> wrote: Yes apparently the code run by the test uses the progress bar (do display progress) and at some point put its max to 1 (since there is only one definition to process) => boum.
since in that case we get a zeroDivide.
Stef
On Nov 3, 2012, at 9:51 PM, Stéphane Ducasse wrote:
I'm wondering if the progress bar code is not bogus and that it is created with wrong values.
Apparently there was failing test which invoked a on:fork:â¦.
This is strange since the processTest were red. Now running them alone brings green tests.
<Screen Shot 2012-11-03 at 7.42.47 PM.pdf> Stef On Nov 3, 2012, at 12:53 PM, Stéphane Ducasse wrote:
There is a bug blocking the image. SubscriptOutOfBounds: 0
in the SystemProgressMorph class updateJob:
-- Dr. Ciprian TEODOROV Ingénieur Développement CAO
tél : 06 08 54 73 48 mail : ciprian.teodorov@gmail.com www.teodorov.ro
loool... divisionByZero indeed On Sat, Nov 3, 2012 at 10:52 PM, Stéphane Ducasse <stephane.ducasse@inria.fr
wrote:
SystemProgressMorph show: 'Doing...' from: 0 to: 1 during: [ :bar | 1 to: 2 do: [ :x | bar current: x. (Delay forMilliseconds: 20) wait "Just to slow it down so we can see what's going on" ] ].
Works while
SystemProgressMorph show: 'Doing...' from: 1 to: 1 during: [ :bar | 1 to: 2 do: [ :x | bar current: x. (Delay forMilliseconds: 20) wait "Just to slow it down so we can see what's going on" ] ].
not :)
Stef
On Nov 3, 2012, at 10:12 PM, Ciprian Teodorov wrote:
Hi Stef,
I have seen this problem too, and I think that JobTest is guilty for this behaviour, since its tearDown method removes all bars, even though they are not his to remove. Try removing the tearDown... it work in my case
JobTest>>tearDown SystemProgressMorph uniqueInstance bars do: [ :e | e close ].
I hope that this helps...
If not I think that there is a problem with the SystemProgressMorph api in a concurrent context... An arbitrary class should not be able to remove bars from our nice progress morph...
Cheers, Ciprian
On Sat, Nov 3, 2012 at 9:57 PM, Stéphane Ducasse < stephane.ducasse@inria.fr> wrote: Yes apparently the code run by the test uses the progress bar (do display progress) and at some point put its max to 1 (since there is only one definition to process) => boum.
since in that case we get a zeroDivide.
Stef
On Nov 3, 2012, at 9:51 PM, Stéphane Ducasse wrote:
I'm wondering if the progress bar code is not bogus and that it is created with wrong values.
Apparently there was failing test which invoked a on:fork:â¦.
This is strange since the processTest were red. Now running them alone brings green tests.
<Screen Shot 2012-11-03 at 7.42.47 PM.pdf> Stef On Nov 3, 2012, at 12:53 PM, Stéphane Ducasse wrote:
There is a bug blocking the image. SubscriptOutOfBounds: 0
in the SystemProgressMorph class updateJob:
-- Dr. Ciprian TEODOROV Ingénieur Développement CAO
tél : 06 08 54 73 48 mail : ciprian.teodorov@gmail.com www.teodorov.ro
-- Dr. Ciprian TEODOROV Ingénieur Développement CAO tél : 06 08 54 73 48 mail : ciprian.teodorov@gmail.com www.teodorov.ro
participants (4)
-
Camillo Bruni -
Ciprian Teodorov -
stephane ducasse -
Stéphane Ducasse