[ltt-dev] [UST PATCH] TESTS: Add a delay at the start of the fork test for a more uniform testing.

[ltt-dev] [UST PATCH] TESTS: Add a delay at the start of the fork test for a more uniform testing.

[Yannick Brosseau]

Add a sleep to leave time for the ustconsumer thread to initialize correctly
before the fork.
Most of the time the consumer was not yet started at the time of the fork so
a bunch of initializations were not done and the fork code path was not
tested properly.

Signed-off-by: Yannick Brosseau <yannick.brosseau at gmail.com>
---
 tests/fork/fork.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/tests/fork/fork.c b/tests/fork/fork.c
index 5ce3d6d..a80518d 100644
--- a/tests/fork/fork.c
+++ b/tests/fork/fork.c
@@ -34,6 +34,9 @@ int main(int argc, char **argv, char *env[])
 	printf("Fork test program, parent pid is %d\n", getpid());
 	trace_mark(ust, before_fork, MARK_NOARGS);
 
+	/* Sleep here to make sure the consumer is initialized before we fork */
+	sleep(1);
+
 	result = fork();
 	if(result == -1) {
 		perror("fork");
-- 
1.7.2.3

[ltt-dev] [UST PATCH] TESTS: Add a delay at the start of the fork test for a more uniform testing.

[Nils Carlson]

Yannick Brosseau wrote:
> Add a sleep to leave time for the ustconsumer thread to initialize correctly
> before the fork.
> Most of the time the consumer was not yet started at the time of the fork so
> a bunch of initializations were not done and the fork code path was not
> tested properly.
>
>   
I'm afraid we're falling into a bit of a trap here. The trap is called
"fixing the test-case instead of fixing the code". It's quite typical
for an application to fork right after startup. In fact, this is probably
by far the most common place where applications fork when going into a 
daemon
mode. So we should probably focus on fixing the problem instead, probably by
detecting that we are following forks with UST and if we are somehow slowing
down the initialisation functions so that everything has started up properly
by the time we enter main().

/Nils

> Signed-off-by: Yannick Brosseau <yannick.brosseau at gmail.com>
> ---
>  tests/fork/fork.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/tests/fork/fork.c b/tests/fork/fork.c
> index 5ce3d6d..a80518d 100644
> --- a/tests/fork/fork.c
> +++ b/tests/fork/fork.c
> @@ -34,6 +34,9 @@ int main(int argc, char **argv, char *env[])
>  	printf("Fork test program, parent pid is %d\n", getpid());
>  	trace_mark(ust, before_fork, MARK_NOARGS);
>  
> +	/* Sleep here to make sure the consumer is initialized before we fork */
> +	sleep(1);
> +
>  	result = fork();
>  	if(result == -1) {
>  		perror("fork");
>   

[ltt-dev] [UST PATCH] TESTS: Add a delay at the start of the fork test for a more uniform testing.

[Yannick Brosseau]

On 2011-02-22 03:28, Nils Carlson wrote:
> Yannick Brosseau wrote:
>> Add a sleep to leave time for the ustconsumer thread to initialize
>> correctly
>> before the fork.
>> Most of the time the consumer was not yet started at the time of the
>> fork so
>> a bunch of initializations were not done and the fork code path was not
>> tested properly.
>>
>>   
> I'm afraid we're falling into a bit of a trap here. The trap is called
> "fixing the test-case instead of fixing the code". It's quite typical
No that's not the case here. I'm fixing the test case, so it shows us a
problem that was occuring randomly before.
With my "fix" to the test case, the test case will fail all the time and
showing us the problem when we fork later. (Some application do it, so
UST should not fail there)

I could duplicate the test case with one doing an early fork and one
doing a late fork. I did not do it, because there was no problem to the
early fork.

The fix to the problem is my other email.

> for an application to fork right after startup. In fact, this is probably
> by far the most common place where applications fork when going into a
> daemon
> mode. So we should probably focus on fixing the problem instead,
> probably by
> detecting that we are following forks with UST and if we are somehow
> slowing
> down the initialisation functions so that everything has started up
> properly
> by the time we enter main().
>
> /Nils
>
>> Signed-off-by: Yannick Brosseau <yannick.brosseau at gmail.com>
>> ---
>>  tests/fork/fork.c |    3 +++
>>  1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> diff --git a/tests/fork/fork.c b/tests/fork/fork.c
>> index 5ce3d6d..a80518d 100644
>> --- a/tests/fork/fork.c
>> +++ b/tests/fork/fork.c
>> @@ -34,6 +34,9 @@ int main(int argc, char **argv, char *env[])
>>      printf("Fork test program, parent pid is %d\n", getpid());
>>      trace_mark(ust, before_fork, MARK_NOARGS);
>>  
>> +    /* Sleep here to make sure the consumer is initialized before we
>> fork */
>> +    sleep(1);
>> +
>>      result = fork();
>>      if(result == -1) {
>>          perror("fork");
>>   
>

[ltt-dev] [UST PATCH] TESTS: Add a delay at the start of the fork test for a more uniform testing.

[Nils Carlson]

Yannick Brosseau wrote:
> On 2011-02-22 03:28, Nils Carlson wrote:
>   
>> Yannick Brosseau wrote:
>>     
>>> Add a sleep to leave time for the ustconsumer thread to initialize
>>> correctly
>>> before the fork.
>>> Most of the time the consumer was not yet started at the time of the
>>> fork so
>>> a bunch of initializations were not done and the fork code path was not
>>> tested properly.
>>>
>>>   
>>>       
>> I'm afraid we're falling into a bit of a trap here. The trap is called
>> "fixing the test-case instead of fixing the code". It's quite typical
>>     
> No that's not the case here. I'm fixing the test case, so it shows us a
> problem that was occuring randomly before.
> With my "fix" to the test case, the test case will fail all the time and
> showing us the problem when we fork later. (Some application do it, so
> UST should not fail there)
>   
So you're saying that this causes the test case to fail all the time 
because now we leave time for things to properly initialise, while if we 
don't initialise things sometimes work?
I think I understand what you mean but I still don't agree. What about 
making sure somehow that we are fully initialized before we hit main()? 
In the case where we are tracing from startup using usttrace we should 
in fact be fully initialised before we allow the program to enter main.

If you don't think we should be fully initialized before entering main() 
please explain why.

/Nils
> I could duplicate the test case with one doing an early fork and one
> doing a late fork. I did not do it, because there was no problem to the
> early fork.
>
> The fix to the problem is my other email.
>
>   
>> for an application to fork right after startup. In fact, this is probably
>> by far the most common place where applications fork when going into a
>> daemon
>> mode. So we should probably focus on fixing the problem instead,
>> probably by
>> detecting that we are following forks with UST and if we are somehow
>> slowing
>> down the initialisation functions so that everything has started up
>> properly
>> by the time we enter main().
>>
>> /Nils
>>
>>     
>>> Signed-off-by: Yannick Brosseau <yannick.brosseau at gmail.com>
>>> ---
>>>  tests/fork/fork.c |    3 +++
>>>  1 files changed, 3 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/tests/fork/fork.c b/tests/fork/fork.c
>>> index 5ce3d6d..a80518d 100644
>>> --- a/tests/fork/fork.c
>>> +++ b/tests/fork/fork.c
>>> @@ -34,6 +34,9 @@ int main(int argc, char **argv, char *env[])
>>>      printf("Fork test program, parent pid is %d\n", getpid());
>>>      trace_mark(ust, before_fork, MARK_NOARGS);
>>>  
>>> +    /* Sleep here to make sure the consumer is initialized before we
>>> fork */
>>> +    sleep(1);
>>> +
>>>      result = fork();
>>>      if(result == -1) {
>>>          perror("fork");
>>>   
>>>       
>
>   

[ltt-dev] [UST PATCH] TESTS: Add a delay at the start of the fork test for a more uniform testing.

[Yannick Brosseau]

>>>>         
>>> I'm afraid we're falling into a bit of a trap here. The trap is called
>>> "fixing the test-case instead of fixing the code". It's quite typical
>>>     
>> No that's not the case here. I'm fixing the test case, so it shows us a
>> problem that was occuring randomly before.
>> With my "fix" to the test case, the test case will fail all the time and
>> showing us the problem when we fork later. (Some application do it, so
>> UST should not fail there)
>>   
> So you're saying that this causes the test case to fail all the time
> because now we leave time for things to properly initialise, while if
> we don't initialise things sometimes work?
> I think I understand what you mean but I still don't agree. What about
> making sure somehow that we are fully initialized before we hit
> main()? In the case where we are tracing from startup using usttrace
> we should in fact be fully initialised before we allow the program to
> enter main.
>
> If you don't think we should be fully initialized before entering
> main() please explain why.

I have no real opinion on the initialization, I don't have enough
knowledge about it right now.
But I see no real needs for the consumerd to be started before we start
main. I can imagine cases where we want to get to main the fastest way
possible and leave the unnecessary things for later.

That being said, my goal right now it to get a test suite that do useful
tests and can identify problems and the change I propose to the test are
finding problems.

Also, if we decide that we need to be fully initialized, we will need to
write a test case that can test that.

[ltt-dev] [UST PATCH] TESTS: Add a delay at the start of the fork test for a more uniform testing.

[Mathieu Desnoyers]

* Yannick Brosseau (yannick.brosseau at gmail.com) wrote:
> 
> >>>>         
> >>> I'm afraid we're falling into a bit of a trap here. The trap is called
> >>> "fixing the test-case instead of fixing the code". It's quite typical
> >>>     
> >> No that's not the case here. I'm fixing the test case, so it shows us a
> >> problem that was occuring randomly before.
> >> With my "fix" to the test case, the test case will fail all the time and
> >> showing us the problem when we fork later. (Some application do it, so
> >> UST should not fail there)
> >>   
> > So you're saying that this causes the test case to fail all the time
> > because now we leave time for things to properly initialise, while if
> > we don't initialise things sometimes work?
> > I think I understand what you mean but I still don't agree. What about
> > making sure somehow that we are fully initialized before we hit
> > main()? In the case where we are tracing from startup using usttrace
> > we should in fact be fully initialised before we allow the program to
> > enter main.
> >
> > If you don't think we should be fully initialized before entering
> > main() please explain why.
> 
> I have no real opinion on the initialization, I don't have enough
> knowledge about it right now.
> But I see no real needs for the consumerd to be started before we start
> main. I can imagine cases where we want to get to main the fastest way
> possible and leave the unnecessary things for later.
> 
> That being said, my goal right now it to get a test suite that do useful
> tests and can identify problems and the change I propose to the test are
> finding problems.
> 
> Also, if we decide that we need to be fully initialized, we will need to
> write a test case that can test that.

Exactly. I think we need to test:

- case where app is started slightly before consumerd
- case where app is started after consumerd

Thanks,

Mathieu

> 
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

[ltt-dev] [UST PATCH] TESTS: Add a delay at the start of the fork test for a more uniform testing.

[Nils Carlson]

Yannick Brosseau wrote:
> Add a sleep to leave time for the ustconsumer thread to initialize correctly
> before the fork.
> Most of the time the consumer was not yet started at the time of the fork so
> a bunch of initializations were not done and the fork code path was not
> tested properly.
>
> Signed-off-by: Yannick Brosseau <yannick.brosseau at gmail.com>
>   
    Acked-by: Nils Carlson <nils.carlson at ericsson.com>

> ---
>  tests/fork/fork.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/tests/fork/fork.c b/tests/fork/fork.c
> index 5ce3d6d..a80518d 100644
> --- a/tests/fork/fork.c
> +++ b/tests/fork/fork.c
> @@ -34,6 +34,9 @@ int main(int argc, char **argv, char *env[])
>  	printf("Fork test program, parent pid is %d\n", getpid());
>  	trace_mark(ust, before_fork, MARK_NOARGS);
>  
> +	/* Sleep here to make sure the consumer is initialized before we fork */
> +	sleep(1);
> +
>  	result = fork();
>  	if(result == -1) {
>  		perror("fork");
>