Re: [E-devel] [EGIT] [core/efl] master 01/01: tests: ecore: relax the timing precision for the promise timeout test

2016-09-20 Thread Stefan Schmidt
Hello.

On 19/09/16 15:31, Tom Hacohen wrote:
> On 19/09/16 14:01, Stefan Schmidt wrote:
>> Hello.
>>
>> On 19/09/16 14:18, Tom Hacohen wrote:
>>> On 19/09/16 13:08, Stefan Schmidt wrote:
 stefan pushed a commit to branch master.

 http://git.enlightenment.org/core/efl.git/commit/?id=c25d4e8325b428122439860f9d49dd25a4b4b66d

 commit c25d4e8325b428122439860f9d49dd25a4b4b66d
 Author: Stefan Schmidt 
 Date:   Mon Sep 19 14:01:19 2016 +0200

 tests: ecore: relax the timing precision for the promise timeout test

 This test has been failing on Jenkins again and again. After adding 
 the debug
 a while ago it now shows that the value is between 0.01 and 0.02 in 
 all cases
 I have seen. Relaxing the timeout here a bit to make it pass in 
 situation where
 our CI is under load.
 ---
  src/tests/ecore/ecore_test_timer.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

 diff --git a/src/tests/ecore/ecore_test_timer.c 
 b/src/tests/ecore/ecore_test_timer.c
 index c7547e4..f3b277b 100644
 --- a/src/tests/ecore/ecore_test_timer.c
 +++ b/src/tests/ecore/ecore_test_timer.c
 @@ -183,8 +183,8 @@ _ecore_promise_quit(void *data, const Efl_Event *ev)
 double *start = success->value;
 double delta = ecore_loop_time_get() - *start;

 -   fprintf(stderr, "Ecore promise timeout took %f (should be <= 0.01)\n", 
 delta - 0.2);
 -   fail_if(delta - 0.2 > 0.01);
 +   fprintf(stderr, "Ecore promise timeout took %f (should be <= 0.02)\n", 
 delta - 0.2);
 +   fail_if(delta - 0.2 > 0.02);

 *bob = EINA_TRUE;
 ecore_main_loop_quit();

>>>
>>>
>>> Why is there an fprintf there? We don't do it in any other tests. That
>>> text should be either removed or moved to a comment. No?
>>
>> I already added it before this commit. For the exact reason to get some
>> debug information while this runs on Jenkins (where the problem happens).
>>
>> I left it in to verify that all works as expect for a while and after
>> that it can go.
>
> You have ck_assert_int_le() (or something like that) for that. It will
> print the values when it fails.
>
> for example:
>
> ck_assert_int_le(delta - 0.2, 0.02);

This would not work as ck_assert_int_* deals with signed integers and 
our values are double here.

I switched to ck_assert_msg though which combines the failure handling 
as well as messaging printing in the failure case.

regards
Stefan Schmidt

--
___
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel


Re: [E-devel] [EGIT] [core/efl] master 01/01: tests: ecore: relax the timing precision for the promise timeout test

2016-09-20 Thread Tom Hacohen
On 20/09/16 00:30, Carsten Haitzler (The Rasterman) wrote:
> On Mon, 19 Sep 2016 13:18:47 +0100 Tom Hacohen  said:
>
>> On 19/09/16 13:08, Stefan Schmidt wrote:
>>> stefan pushed a commit to branch master.
>>>
>>> http://git.enlightenment.org/core/efl.git/commit/?id=c25d4e8325b428122439860f9d49dd25a4b4b66d
>>>
>>> commit c25d4e8325b428122439860f9d49dd25a4b4b66d
>>> Author: Stefan Schmidt 
>>> Date:   Mon Sep 19 14:01:19 2016 +0200
>>>
>>> tests: ecore: relax the timing precision for the promise timeout test
>>>
>>> This test has been failing on Jenkins again and again. After adding the
>>> debug a while ago it now shows that the value is between 0.01 and 0.02 in
>>> all cases I have seen. Relaxing the timeout here a bit to make it pass in
>>> situation where our CI is under load.
>>> ---
>>>  src/tests/ecore/ecore_test_timer.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/tests/ecore/ecore_test_timer.c
>>> b/src/tests/ecore/ecore_test_timer.c index c7547e4..f3b277b 100644
>>> --- a/src/tests/ecore/ecore_test_timer.c
>>> +++ b/src/tests/ecore/ecore_test_timer.c
>>> @@ -183,8 +183,8 @@ _ecore_promise_quit(void *data, const Efl_Event *ev)
>>> double *start = success->value;
>>> double delta = ecore_loop_time_get() - *start;
>>>
>>> -   fprintf(stderr, "Ecore promise timeout took %f (should be <= 0.01)\n",
>>> delta - 0.2);
>>> -   fail_if(delta - 0.2 > 0.01);
>>> +   fprintf(stderr, "Ecore promise timeout took %f (should be <= 0.02)\n",
>>> delta - 0.2);
>>> +   fail_if(delta - 0.2 > 0.02);
>>>
>>> *bob = EINA_TRUE;
>>> ecore_main_loop_quit();
>>>
>>
>>
>> Why is there an fprintf there? We don't do it in any other tests. That
>> text should be either removed or moved to a comment. No?
>
> we do this in lots of tests. well printfs. also eina log's too. i make use of
> them all the time to run a test by hand to see what is actually going on in 
> the
> test.
>
>

It's useful in some cases, where you want to print additional 
information, but as I said, in this case is not, because all of the 
information would be printed when the error case actually happens if the 
check is done correctly.

--
Tom.

--
___
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel


Re: [E-devel] [EGIT] [core/efl] master 01/01: tests: ecore: relax the timing precision for the promise timeout test

2016-09-19 Thread The Rasterman
On Mon, 19 Sep 2016 13:18:47 +0100 Tom Hacohen  said:

> On 19/09/16 13:08, Stefan Schmidt wrote:
> > stefan pushed a commit to branch master.
> >
> > http://git.enlightenment.org/core/efl.git/commit/?id=c25d4e8325b428122439860f9d49dd25a4b4b66d
> >
> > commit c25d4e8325b428122439860f9d49dd25a4b4b66d
> > Author: Stefan Schmidt 
> > Date:   Mon Sep 19 14:01:19 2016 +0200
> >
> > tests: ecore: relax the timing precision for the promise timeout test
> >
> > This test has been failing on Jenkins again and again. After adding the
> > debug a while ago it now shows that the value is between 0.01 and 0.02 in
> > all cases I have seen. Relaxing the timeout here a bit to make it pass in
> > situation where our CI is under load.
> > ---
> >  src/tests/ecore/ecore_test_timer.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/tests/ecore/ecore_test_timer.c
> > b/src/tests/ecore/ecore_test_timer.c index c7547e4..f3b277b 100644
> > --- a/src/tests/ecore/ecore_test_timer.c
> > +++ b/src/tests/ecore/ecore_test_timer.c
> > @@ -183,8 +183,8 @@ _ecore_promise_quit(void *data, const Efl_Event *ev)
> > double *start = success->value;
> > double delta = ecore_loop_time_get() - *start;
> >
> > -   fprintf(stderr, "Ecore promise timeout took %f (should be <= 0.01)\n",
> > delta - 0.2);
> > -   fail_if(delta - 0.2 > 0.01);
> > +   fprintf(stderr, "Ecore promise timeout took %f (should be <= 0.02)\n",
> > delta - 0.2);
> > +   fail_if(delta - 0.2 > 0.02);
> >
> > *bob = EINA_TRUE;
> > ecore_main_loop_quit();
> >
> 
> 
> Why is there an fprintf there? We don't do it in any other tests. That 
> text should be either removed or moved to a comment. No?

we do this in lots of tests. well printfs. also eina log's too. i make use of
them all the time to run a test by hand to see what is actually going on in the
test.


-- 
- Codito, ergo sum - "I code, therefore I am" --
The Rasterman (Carsten Haitzler)ras...@rasterman.com


--
___
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel


Re: [E-devel] [EGIT] [core/efl] master 01/01: tests: ecore: relax the timing precision for the promise timeout test

2016-09-19 Thread Tom Hacohen
On 19/09/16 14:01, Stefan Schmidt wrote:
> Hello.
>
> On 19/09/16 14:18, Tom Hacohen wrote:
>> On 19/09/16 13:08, Stefan Schmidt wrote:
>>> stefan pushed a commit to branch master.
>>>
>>> http://git.enlightenment.org/core/efl.git/commit/?id=c25d4e8325b428122439860f9d49dd25a4b4b66d
>>>
>>> commit c25d4e8325b428122439860f9d49dd25a4b4b66d
>>> Author: Stefan Schmidt 
>>> Date:   Mon Sep 19 14:01:19 2016 +0200
>>>
>>> tests: ecore: relax the timing precision for the promise timeout test
>>>
>>> This test has been failing on Jenkins again and again. After adding the 
>>> debug
>>> a while ago it now shows that the value is between 0.01 and 0.02 in all 
>>> cases
>>> I have seen. Relaxing the timeout here a bit to make it pass in 
>>> situation where
>>> our CI is under load.
>>> ---
>>>  src/tests/ecore/ecore_test_timer.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/tests/ecore/ecore_test_timer.c 
>>> b/src/tests/ecore/ecore_test_timer.c
>>> index c7547e4..f3b277b 100644
>>> --- a/src/tests/ecore/ecore_test_timer.c
>>> +++ b/src/tests/ecore/ecore_test_timer.c
>>> @@ -183,8 +183,8 @@ _ecore_promise_quit(void *data, const Efl_Event *ev)
>>> double *start = success->value;
>>> double delta = ecore_loop_time_get() - *start;
>>>
>>> -   fprintf(stderr, "Ecore promise timeout took %f (should be <= 0.01)\n", 
>>> delta - 0.2);
>>> -   fail_if(delta - 0.2 > 0.01);
>>> +   fprintf(stderr, "Ecore promise timeout took %f (should be <= 0.02)\n", 
>>> delta - 0.2);
>>> +   fail_if(delta - 0.2 > 0.02);
>>>
>>> *bob = EINA_TRUE;
>>> ecore_main_loop_quit();
>>>
>>
>>
>> Why is there an fprintf there? We don't do it in any other tests. That
>> text should be either removed or moved to a comment. No?
>
> I already added it before this commit. For the exact reason to get some
> debug information while this runs on Jenkins (where the problem happens).
>
> I left it in to verify that all works as expect for a while and after
> that it can go.

You have ck_assert_int_le() (or something like that) for that. It will 
print the values when it fails.

for example:

ck_assert_int_le(delta - 0.2, 0.02);

Had the original author used that, you wouldn't have needed to even 
change the code.

Just a reminder for all of us (myself included), to use those macros more.

--
Tom.

--
Tom.


--
___
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel


Re: [E-devel] [EGIT] [core/efl] master 01/01: tests: ecore: relax the timing precision for the promise timeout test

2016-09-19 Thread Stefan Schmidt
Hello.

On 19/09/16 14:18, Tom Hacohen wrote:
> On 19/09/16 13:08, Stefan Schmidt wrote:
>> stefan pushed a commit to branch master.
>>
>> http://git.enlightenment.org/core/efl.git/commit/?id=c25d4e8325b428122439860f9d49dd25a4b4b66d
>>
>> commit c25d4e8325b428122439860f9d49dd25a4b4b66d
>> Author: Stefan Schmidt 
>> Date:   Mon Sep 19 14:01:19 2016 +0200
>>
>> tests: ecore: relax the timing precision for the promise timeout test
>>
>> This test has been failing on Jenkins again and again. After adding the 
>> debug
>> a while ago it now shows that the value is between 0.01 and 0.02 in all 
>> cases
>> I have seen. Relaxing the timeout here a bit to make it pass in 
>> situation where
>> our CI is under load.
>> ---
>>  src/tests/ecore/ecore_test_timer.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/tests/ecore/ecore_test_timer.c 
>> b/src/tests/ecore/ecore_test_timer.c
>> index c7547e4..f3b277b 100644
>> --- a/src/tests/ecore/ecore_test_timer.c
>> +++ b/src/tests/ecore/ecore_test_timer.c
>> @@ -183,8 +183,8 @@ _ecore_promise_quit(void *data, const Efl_Event *ev)
>> double *start = success->value;
>> double delta = ecore_loop_time_get() - *start;
>>
>> -   fprintf(stderr, "Ecore promise timeout took %f (should be <= 0.01)\n", 
>> delta - 0.2);
>> -   fail_if(delta - 0.2 > 0.01);
>> +   fprintf(stderr, "Ecore promise timeout took %f (should be <= 0.02)\n", 
>> delta - 0.2);
>> +   fail_if(delta - 0.2 > 0.02);
>>
>> *bob = EINA_TRUE;
>> ecore_main_loop_quit();
>>
>
>
> Why is there an fprintf there? We don't do it in any other tests. That
> text should be either removed or moved to a comment. No?

I already added it before this commit. For the exact reason to get some 
debug information while this runs on Jenkins (where the problem happens).

I left it in to verify that all works as expect for a while and after 
that it can go.

regards
Stefan Schmidt

--
___
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel


Re: [E-devel] [EGIT] [core/efl] master 01/01: tests: ecore: relax the timing precision for the promise timeout test

2016-09-19 Thread Tom Hacohen
On 19/09/16 13:08, Stefan Schmidt wrote:
> stefan pushed a commit to branch master.
>
> http://git.enlightenment.org/core/efl.git/commit/?id=c25d4e8325b428122439860f9d49dd25a4b4b66d
>
> commit c25d4e8325b428122439860f9d49dd25a4b4b66d
> Author: Stefan Schmidt 
> Date:   Mon Sep 19 14:01:19 2016 +0200
>
> tests: ecore: relax the timing precision for the promise timeout test
>
> This test has been failing on Jenkins again and again. After adding the 
> debug
> a while ago it now shows that the value is between 0.01 and 0.02 in all 
> cases
> I have seen. Relaxing the timeout here a bit to make it pass in situation 
> where
> our CI is under load.
> ---
>  src/tests/ecore/ecore_test_timer.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/tests/ecore/ecore_test_timer.c 
> b/src/tests/ecore/ecore_test_timer.c
> index c7547e4..f3b277b 100644
> --- a/src/tests/ecore/ecore_test_timer.c
> +++ b/src/tests/ecore/ecore_test_timer.c
> @@ -183,8 +183,8 @@ _ecore_promise_quit(void *data, const Efl_Event *ev)
> double *start = success->value;
> double delta = ecore_loop_time_get() - *start;
>
> -   fprintf(stderr, "Ecore promise timeout took %f (should be <= 0.01)\n", 
> delta - 0.2);
> -   fail_if(delta - 0.2 > 0.01);
> +   fprintf(stderr, "Ecore promise timeout took %f (should be <= 0.02)\n", 
> delta - 0.2);
> +   fail_if(delta - 0.2 > 0.02);
>
> *bob = EINA_TRUE;
> ecore_main_loop_quit();
>


Why is there an fprintf there? We don't do it in any other tests. That 
text should be either removed or moved to a comment. No?

--
Tom.

--
___
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel


[EGIT] [core/efl] master 01/01: tests: ecore: relax the timing precision for the promise timeout test

2016-09-19 Thread Stefan Schmidt
stefan pushed a commit to branch master.

http://git.enlightenment.org/core/efl.git/commit/?id=c25d4e8325b428122439860f9d49dd25a4b4b66d

commit c25d4e8325b428122439860f9d49dd25a4b4b66d
Author: Stefan Schmidt 
Date:   Mon Sep 19 14:01:19 2016 +0200

tests: ecore: relax the timing precision for the promise timeout test

This test has been failing on Jenkins again and again. After adding the 
debug
a while ago it now shows that the value is between 0.01 and 0.02 in all 
cases
I have seen. Relaxing the timeout here a bit to make it pass in situation 
where
our CI is under load.
---
 src/tests/ecore/ecore_test_timer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/tests/ecore/ecore_test_timer.c 
b/src/tests/ecore/ecore_test_timer.c
index c7547e4..f3b277b 100644
--- a/src/tests/ecore/ecore_test_timer.c
+++ b/src/tests/ecore/ecore_test_timer.c
@@ -183,8 +183,8 @@ _ecore_promise_quit(void *data, const Efl_Event *ev)
double *start = success->value;
double delta = ecore_loop_time_get() - *start;
 
-   fprintf(stderr, "Ecore promise timeout took %f (should be <= 0.01)\n", 
delta - 0.2);
-   fail_if(delta - 0.2 > 0.01);
+   fprintf(stderr, "Ecore promise timeout took %f (should be <= 0.02)\n", 
delta - 0.2);
+   fail_if(delta - 0.2 > 0.02);
 
*bob = EINA_TRUE;
ecore_main_loop_quit();

--