Smalltalk › Pharo › Pharo Smalltalk Developers

Green tests

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

10 messages Options

Guillermo Polito

Green tests

Hi all,

In a couple of minutes I've made some (not complicated) fixes to make Rubric tests green again:

https://github.com/pharo-project/pharo/pull/3129

This is of course not a super Rubric enhancement (for which I've opened another issue requesting some missing feature https://github.com/pharo-project/pharo/issues/3128) but this allows us to move on with a better infrastructure :).

There are still some some tests that fail from time to time, related to system announcers and finalization. See https://github.com/pharo-project/pharo/issues/2471.

Though I've spent half a day yesterday and half today trying to track it down, I don't yet have a clear way of reproducing it. I've read the entire code that touches this part of the system and I really bet this is a race condition :).

So any second pair of eyes here would really be helpful!

Guille

ducasse

Re: Green tests

Cool and thanks!

I love this green bar and I will try to keep it green.

Do you know if the test writing by pierre should be pushed in Pharo because I have the impression that they will be red first

Hi all,

In a couple of minutes I've made some (not complicated) fixes to make Rubric tests green again:
https://github.com/pharo-project/pharo/pull/3129

I will check them.

We got some tests for maxlength and started with the stupid point.

This is of course not a super Rubric enhancement (for which I've opened another issue requesting some missing feature https://github.com/pharo-project/pharo/issues/3128) but this allows us to move on with a better infrastructure :).

There are still some some tests that fail from time to time, related to system announcers and finalization. See https://github.com/pharo-project/pharo/issues/2471.
Though I've spent half a day yesterday and half today trying to track it down, I don't yet have a clear way of reproducing it. I've read the entire code that touches this part of the system and I really bet this is a race condition :).
So any second pair of eyes here would really be helpful!

I’m inherently race condition incompatible and just avoid concurrent programming because I’m too dull on this.

Guille

Marcus Denker-4

Re: Green tests

> On 2 Apr 2019, at 17:30, ducasse <[hidden email]> wrote:
>
> Cool and thanks!
> I love this green bar and I will try to keep it green.
> Do you know if the test writing by pierre should be pushed in Pharo because I have the impression that they will be red first
>
If we add failing tests, then *everyone* is impacted. Instead of “ah, green” we will again have to manually check
*every* *single* Pull request. And we, again, will forget to do it. And thus break even more.

I really think keeping the tests green is very, very much better for everyone.

Marcus

Guillermo Polito

Re: Green tests

In reply to this post by ducasse

On Tue, Apr 2, 2019 at 5:31 PM ducasse <[hidden email]> wrote:

Cool and thanks!
I love this green bar and I will try to keep it green.
Do you know if the test writing by pierre should be pushed in Pharo because I have the impression that they will be red first

Short answer: no :).

Long answer:

I've made a review and requested changes. The issue is related to the need of the new VM.

The main problem is that the only way to test it properly is to open a file, closing it and ask your system if the file is still open (which is OS dependent e.g., calling lsof on unix/osx, or the equivalent on windows).

The second better test we can do is to try to open as many files as the limit and check that we cannot open more files or that we do not segfault :) but it's not super fun when debugging.

Maybe we can just close/ignore that PR?

Hi all,

In a couple of minutes I've made some (not complicated) fixes to make Rubric tests green again:
https://github.com/pharo-project/pharo/pull/3129

I will check them.
We got some tests for maxlength and started with the stupid point.

Yeah, we have to fix some ugly stuff on it (the centered ghost text, the max length...), but there is "no rush" :).

This is of course not a super Rubric enhancement (for which I've opened another issue requesting some missing feature https://github.com/pharo-project/pharo/issues/3128) but this allows us to move on with a better infrastructure :).

There are still some some tests that fail from time to time, related to system announcers and finalization. See https://github.com/pharo-project/pharo/issues/2471.
Though I've spent half a day yesterday and half today trying to track it down, I don't yet have a clear way of reproducing it. I've read the entire code that touches this part of the system and I really bet this is a race condition :).
So any second pair of eyes here would really be helpful!

I’m inherently race condition incompatible and just avoid concurrent programming because I’m too dull on this.

Even funnier is that I think the race condition happens during the stage loading BaselineOfPharo. It's a super huge step, and we need better debugging tools for that kind of scenarios...

Guille

Guille Polito

Research Engineer

Centre de Recherche en Informatique, Signal et Automatique de Lille

CRIStAL - UMR 9189

French National Center for Scientific Research - http://www.cnrs.fr

Web: http://guillep.github.io

Phone: +33 06 52 70 66 13

Guillermo Polito

Re: Green tests

In reply to this post by Marcus Denker-4

On Tue, Apr 2, 2019 at 5:36 PM Marcus Denker <[hidden email]> wrote:

> On 2 Apr 2019, at 17:30, ducasse <[hidden email]> wrote:
>
> Cool and thanks!
> I love this green bar and I will try to keep it green.
> Do you know if the test writing by pierre should be pushed in Pharo because I have the impression that they will be red first
>
If we add failing tests, then *everyone* is impacted. Instead of “ah, green” we will again have to manually check
*every* *single* Pull request. And we, again, will forget to do it. And thus break even more.

I really think keeping the tests green is very, very much better for everyone.

Yes Marcus, but there is another "hidden" point in my mail: my fix was really simple, anybody could have done it.

- just complaining does not fix the build.

- removing breaking tests will not guarantee us a better quality either

Marcus

Guille Polito

Research Engineer

Centre de Recherche en Informatique, Signal et Automatique de Lille

CRIStAL - UMR 9189

French National Center for Scientific Research - http://www.cnrs.fr

Web: http://guillep.github.io

Phone: +33 06 52 70 66 13

ducasse

Re: Green tests

And this was not just for fun that tests were broken with rubric.

First there was no tests when Alain fixed rubric

Second some features were bad so not copied and pasted to Rubric.

Life is complex.

And we try our best.

But I did not want to say to Alain: “too bad you work a lot but now we do not care” because we care a lot

about other contributions.

Stef

On 2 Apr 2019, at 17:43, Guillermo Polito <[hidden email]> wrote:

On Tue, Apr 2, 2019 at 5:36 PM Marcus Denker <[hidden email]> wrote:

> On 2 Apr 2019, at 17:30, ducasse <[hidden email]> wrote:
>
> Cool and thanks!
> I love this green bar and I will try to keep it green.
> Do you know if the test writing by pierre should be pushed in Pharo because I have the impression that they will be red first
>
If we add failing tests, then *everyone* is impacted. Instead of “ah, green” we will again have to manually check
*every* *single* Pull request. And we, again, will forget to do it. And thus break even more.

I really think keeping the tests green is very, very much better for everyone.

Yes Marcus, but there is another "hidden" point in my mail: my fix was really simple, anybody could have done it.
So
- just complaining does not fix the build.
- removing breaking tests will not guarantee us a better quality either

Marcus

--

Guille Polito
Research Engineer

Centre de Recherche en Informatique, Signal et Automatique de Lille
CRIStAL - UMR 9189
French National Center for Scientific Research - http://www.cnrs.fr

Web: http://guillep.github.io
Phone: +33 06 52 70 66 13

ducasse

Re: Green tests

In reply to this post by Guillermo Polito

On 2 Apr 2019, at 17:40, Guillermo Polito <[hidden email]> wrote:

On Tue, Apr 2, 2019 at 5:31 PM ducasse <[hidden email]> wrote:
Cool and thanks!
I love this green bar and I will try to keep it green.
Do you know if the test writing by pierre should be pushed in Pharo because I have the impression that they will be red first

Short answer: no :).
Long answer:
I've made a review and requested changes. The issue is related to the need of the new VM.
The main problem is that the only way to test it properly is to open a file, closing it and ask your system if the file is still open (which is OS dependent e.g., calling lsof on unix/osx, or the equivalent on windows).
The second better test we can do is to try to open as many files as the limit and check that we cannot open more files or that we do not segfault :) but it's not super fun when debugging.
Maybe we can just close/ignore that PR?

Just to verify we are talking about: https://github.com/pharo-project/pharo/pull/3100

Then I will close it.

Stef

Hi all,

In a couple of minutes I've made some (not complicated) fixes to make Rubric tests green again:
https://github.com/pharo-project/pharo/pull/3129

I will check them.
We got some tests for maxlength and started with the stupid point.

Yeah, we have to fix some ugly stuff on it (the centered ghost text, the max length...), but there is "no rush" :).

This is of course not a super Rubric enhancement (for which I've opened another issue requesting some missing feature https://github.com/pharo-project/pharo/issues/3128) but this allows us to move on with a better infrastructure :).

There are still some some tests that fail from time to time, related to system announcers and finalization. See https://github.com/pharo-project/pharo/issues/2471.
Though I've spent half a day yesterday and half today trying to track it down, I don't yet have a clear way of reproducing it. I've read the entire code that touches this part of the system and I really bet this is a race condition :).
So any second pair of eyes here would really be helpful!

I’m inherently race condition incompatible and just avoid concurrent programming because I’m too dull on this.

Even funnier is that I think the race condition happens during the stage loading BaselineOfPharo. It's a super huge step, and we need better debugging tools for that kind of scenarios...

Guille

--

Guille Polito
Research Engineer

Centre de Recherche en Informatique, Signal et Automatique de Lille
CRIStAL - UMR 9189
French National Center for Scientific Research - http://www.cnrs.fr

Web: http://guillep.github.io
Phone: +33 06 52 70 66 13

Ben Coman

Re: Green tests

In reply to this post by ducasse

On Tue, 2 Apr 2019 at 23:36, ducasse <[hidden email]> wrote:

I’m inherently race condition incompatible and just avoid concurrent programming because I’m too dull on this.

I've solved a few Pharo race conditions before, so just a tip (that you maybe already know).

You can't troubleshoot a race condition by standard debugging, because it alters the timing.

Best that I've done before is resorting to plain old print-line tracing between every line of the suspect code,

or the equivalent threadsafe collection if its not a hard crash.

e.g....

someMethod

threadsafeCollection add: "methodname-A" , testCondition

original method line 1

threadsafeCollection add: "methodname-B", testCondition

original method line 2

threadsafeCollection add: "methodname-C", testCondition

and then looking for cyclic patterns in that data and breaks in those patterns.

I can have a look in a few weeks. Right now working 13 hour days to the end of the week

and then vacationing in China for 10 days.

Tudor Girba-2

Re: Green tests

Instead of printing, you can also use Beacon. We actually developed Beacon specifically to be able to debug a parallel distributed system.

Cheers,
Doru

> On Apr 2, 2019, at 11:56 PM, Ben Coman <[hidden email]> wrote:
>
>
> On Tue, 2 Apr 2019 at 23:36, ducasse <[hidden email]> wrote:
> I’m inherently race condition incompatible and just avoid concurrent programming because I’m too dull on this.
>
> I've solved a few Pharo race conditions before, so just a tip (that you maybe already know).
> You can't troubleshoot a race condition by standard debugging, because it alters the timing.
> Best that I've done before is resorting to plain old print-line tracing between every line of the suspect code,
> or the equivalent threadsafe collection if its not a hard crash.
> e.g....
> someMethod
> threadsafeCollection add: "methodname-A" , testCondition
> original method line 1
> threadsafeCollection add: "methodname-B", testCondition
> original method line 2
> threadsafeCollection add: "methodname-C", testCondition
>
> and then looking for cyclic patterns in that data and breaks in those patterns.
>
> I can have a look in a few weeks. Right now working 13 hour days to the end of the week
> and then vacationing in China for 10 days.
>

--
www.feenk.com

"Sometimes the best solution is not the best solution."

David T. Lewis

Re: Green tests

Also useful for "quick and dirty" debugging with concurrent processes:

OSProcess trace: 'my debugging note'

This prints to console, identifying the process, receiver, method, and
whatever additional notes you may want to add.

Dave

On Wed, Apr 03, 2019 at 07:34:22AM +0200, Tudor Girba wrote:

> Instead of printing, you can also use Beacon. We actually developed Beacon specifically to be able to debug a parallel distributed system.
>
> Cheers,
> Doru
>
> > On Apr 2, 2019, at 11:56 PM, Ben Coman <[hidden email]> wrote:
> >
> >
> > On Tue, 2 Apr 2019 at 23:36, ducasse <[hidden email]> wrote:
> > I???m inherently race condition incompatible and just avoid concurrent programming because I???m too dull on this.
> >
> > I've solved a few Pharo race conditions before, so just a tip (that you maybe already know).
> > You can't troubleshoot a race condition by standard debugging, because it alters the timing.
> > Best that I've done before is resorting to plain old print-line tracing between every line of the suspect code,
> > or the equivalent threadsafe collection if its not a hard crash.
> > e.g....
> > someMethod
> > threadsafeCollection add: "methodname-A" , testCondition
> > original method line 1
> > threadsafeCollection add: "methodname-B", testCondition
> > original method line 2
> > threadsafeCollection add: "methodname-C", testCondition
> >
> > and then looking for cyclic patterns in that data and breaks in those patterns.
> >
> > I can have a look in a few weeks. Right now working 13 hour days to the end of the week
> > and then vacationing in China for 10 days.
> >
>
> --
> www.feenk.com
>
> "Sometimes the best solution is not the best solution."
>