Hi Hoa Le
I think a problem is that more than one thing is tested in one test case but
this is the legacy code and to fix this is probably out of scope of this ticket.
I think your third option actually is a bit better than just adding a sleep but
you could modify it a little bit by using a much shorter delay (use
osaf_nanosleep() in base/osaf_time.h) and a number of retries that could give a
longer delay if needed. This also gives the possibility to log the specific
event that the retry loop has timed out. Note that you can use the functions
osaf_set_millis_timeout() and osaf_is_timeout() that can be found in the same
header file to create a timed loop instead of using a retry counter.
Example:
struct timespec ts;
osaf_set_millis_timeout(5000, &ts);
while (osaf_is_timeout(ts) == false) {
if (is_my_event() == true) break;
osaf_nanosleep(kHundredMilliseconds);
}
if (osaf_is_timeout(ts) == true) {
printf("my_event() did not happen within reasonable time");
}
I don't think that "base lib" is included in the test code Makefile.am file but
it is in for example the log service api tests (I think it is libopensaf_core
you have to add)
Thanks
Lennart
From: Hoa Le [mailto:[email protected]]
Sent: den 24 april 2018 05:50
To: Lennart Lund <[email protected]>; Anders Widell
<[email protected]>; Hans Nordebäck <[email protected]>
Cc: [email protected]
Subject: Re: [devel] [PATCH 1/1] mds: Correct timing issues in mdstest [#2798]
Hi Lennart,
Thanks for your comments on the ticket, I will update the description of
tet_mds_retrieve_for_change() as your suggestion in the next version of the
patch.
Just have a concern with the comment about sleep(1) in test case
tet_svc_subscr_VDEST_10 (and other).
I agree that we should restrict sleep() in mdstest. But in these particular
test cases, I think sleep() is a suitable approach.
I considered two other alternative approaches for this issue as following, but
they do not seem to be the most fitting options for this.
1) Use tet_mds_retrieve_for_change() (or similar method) to retrieve the event.
The problem is that the tet_mds_retrieve_for_change() does the same thing as
the next steps of this test case (invokes mds_service_retrieve() then verifies
the event by calling tet_verify_version()). So if we use
tet_mds_retrieve_for_change() (or any similar method) here, these next steps of
the test case seem to be meaningless.
2) Start a thread to poll on the selection object of the listening service for
its events. I think this is not a good solution either, since we may catch the
wrong event: "The first service up after successfully subscribing" instead of
"the role change event". To clarify these events, we may have to reuse the
tet_mds_retrieve_for_change() fuction.
Another possible approach is retrying the sequence
mds_service_retrieve/tet_verify_version several times like in the following but
technically, it is the same with the sleep() method.
int num_retries = 0;
while (1) {
if (mds_service_retrieve(gl_tet_adest.mds_pwe1_hdl, 500, SA_DISPATCH_ALL)
!= NCSCC_RC_SUCCESS) {
printf("\nFail to retreive events\n");
FAIL = 1;
break;
}
if ((tet_verify_version(gl_tet_adest.mds_pwe1_hdl, 500, 600, 1,
NCSMDS_NO_ACTIVE)) != 1 ||
(tet_verify_version(gl_tet_adest.mds_pwe1_hdl, 500, 700, 2,
NCSMDS_NO_ACTIVE) != 1)) {
if (num_retries >= 3) {
printf("\nFail to verifying for the versions for NO_ACTIVE
event\n");
FAIL = 1;
break;
} else {
num_retries ++;
sleep(1);
}
} else {
break;
}
}
That was why I have chosen the sleep(1) approach in these test cases. What do
you think about it?
--
Best regards,
Hoa Le
On 04/19/2018 09:34 PM, Lennart Lund wrote:
Hi,
I have reviewed your fix instead of Hans N.
I have a few comments. See the attached diff file. It can be applied on a clone
of your review.
Thanks
Lennart
-----Original Message-----
From: Hoa Le [mailto:[email protected]]
Sent: den 26 mars 2018 06:58
To: Anders Widell
<[email protected]><mailto:[email protected]>; Hans Nordebäck
<[email protected]><mailto:[email protected]>
Cc:
[email protected]<mailto:[email protected]>
Subject: [devel] [PATCH 1/1] mds: Correct timing issues in mdstest [#2798]
In some bad thread scheduling situations, the API service request
on the testing thread may be executed before the corresponding
event being received on the MDS thread. This will lead to
unexpected behavior of the service request and cause the failure
of this test case.
This patch adds a sleep period for mdstest to wait for the expected
event being received on MDS thread before invoking the testing
service request.
---
src/mds/apitest/mdstipc_api.c | 142
++++++++++++++++++++++++++++++------------
1 file changed, 102 insertions(+), 40 deletions(-)
diff --git a/src/mds/apitest/mdstipc_api.c b/src/mds/apitest/mdstipc_api.c
index 669c770..3a6f4ed 100644
--- a/src/mds/apitest/mdstipc_api.c
+++ b/src/mds/apitest/mdstipc_api.c
@@ -1715,6 +1715,7 @@ void tet_svc_subscr_VDEST_10()
FAIL = 1;
}
// Retrieving the events
+ sleep(1);
if (mds_service_retrieve(gl_tet_adest.mds_pwe1_hdl, 500,
SA_DISPATCH_ALL) != NCSCC_RC_SUCCESS)
{
printf("\nRetrieve servive 500
Fail\n");<mailto:);@@-1735,6+1736,7@@voidtet_svc_subscr_VDEST_10()FAIL=1;%7d//Retrievingtheevents+sleep(1);if(mds_service_retrieve(gl_tet_adest.mds_pwe1_hdl,500,SA_DISPATCH_ALL)!=NCSCC_RC_SUCCESS)%7bprintf(>
@@ -1735,6 +1736,7 @@ void
tet_svc_subscr_VDEST_10()<mailto:);@@-1735,6+1736,7@@voidtet_svc_subscr_VDEST_10()FAIL=1;%7d//Retrievingtheevents+sleep(1);if(mds_service_retrieve(gl_tet_adest.mds_pwe1_hdl,500,SA_DISPATCH_ALL)!=NCSCC_RC_SUCCESS)%7bprintf(>
FAIL =
1;<mailto:);@@-1735,6+1736,7@@voidtet_svc_subscr_VDEST_10()FAIL=1;%7d//Retrievingtheevents+sleep(1);if(mds_service_retrieve(gl_tet_adest.mds_pwe1_hdl,500,SA_DISPATCH_ALL)!=NCSCC_RC_SUCCESS)%7bprintf(>
}<mailto:);@@-1735,6+1736,7@@voidtet_svc_subscr_VDEST_10()FAIL=1;%7d//Retrievingtheevents+sleep(1);if(mds_service_retrieve(gl_tet_adest.mds_pwe1_hdl,500,SA_DISPATCH_ALL)!=NCSCC_RC_SUCCESS)%7bprintf(>
// Retrieving the
events<mailto:);@@-1735,6+1736,7@@voidtet_svc_subscr_VDEST_10()FAIL=1;%7d//Retrievingtheevents+sleep(1);if(mds_service_retrieve(gl_tet_adest.mds_pwe1_hdl,500,SA_DISPATCH_ALL)!=NCSCC_RC_SUCCESS)%7bprintf(>
+
sleep(1);<mailto:);@@-1735,6+1736,7@@voidtet_svc_subscr_VDEST_10()FAIL=1;%7d//Retrievingtheevents+sleep(1);if(mds_service_retrieve(gl_tet_adest.mds_pwe1_hdl,500,SA_DISPATCH_ALL)!=NCSCC_RC_SUCCESS)%7bprintf(>
if (mds_service_retrieve(gl_tet_adest.mds_pwe1_hdl,
500,<mailto:);@@-1735,6+1736,7@@voidtet_svc_subscr_VDEST_10()FAIL=1;%7d//Retrievingtheevents+sleep(1);if(mds_service_retrieve(gl_tet_adest.mds_pwe1_hdl,500,SA_DISPATCH_ALL)!=NCSCC_RC_SUCCESS)%7bprintf(>
SA_DISPATCH_ALL) !=
NCSCC_RC_SUCCESS)<mailto:);@@-1735,6+1736,7@@voidtet_svc_subscr_VDEST_10()FAIL=1;%7d//Retrievingtheevents+sleep(1);if(mds_service_retrieve(gl_tet_adest.mds_pwe1_hdl,500,SA_DISPATCH_ALL)!=NCSCC_RC_SUCCESS)%7bprintf(>
{<mailto:);@@-1735,6+1736,7@@voidtet_svc_subscr_VDEST_10()FAIL=1;%7d//Retrievingtheevents+sleep(1);if(mds_service_retrieve(gl_tet_adest.mds_pwe1_hdl,500,SA_DISPATCH_ALL)!=NCSCC_RC_SUCCESS)%7bprintf(>
printf("<mailto:);@@-1735,6+1736,7@@voidtet_svc_subscr_VDEST_10()FAIL=1;%7d//Retrievingtheevents+sleep(1);if(mds_service_retrieve(gl_tet_adest.mds_pwe1_hdl,500,SA_DISPATCH_ALL)!=NCSCC_RC_SUCCESS)%7bprintf(>\nFail
to retreive events\n");
@@ -2001,6 +2003,7 @@ void tet_svc_subscr_VDEST_12()
}
printf("\nAction: Retrieving the events\n");
+ sleep(1);
if (mds_service_retrieve(gl_tet_adest.mds_pwe1_hdl, 500,
SA_DISPATCH_ALL) != NCSCC_RC_SUCCESS)
{
printf("Retrieve Fail\n");
@@ -2165,6 +2168,7 @@ void tet_svc_subscr_ADEST_1()
FAIL = 1;
} else {
printf("\nAction: Retrieve only ONE event\n");
+ sleep(1);
if (mds_service_retrieve(gl_tet_adest.mds_pwe1_hdl, 500,
SA_DISPATCH_ONE) !=
NCSCC_RC_SUCCESS) {
printf("Fail, retrieve ONE\n");
@@ -2492,6 +2496,7 @@ void tet_svc_subscr_ADEST_9()
FAIL = 1;
} else {
printf("\nAction: Retreive three times, third shall fail\n");
+ sleep(1);
if (mds_service_retrieve(gl_tet_adest.mds_pwe1_hdl, 500,
SA_DISPATCH_ONE) !=
NCSCC_RC_SUCCESS) {
printf("\nFail mds_service_retrieve\n");
@@ -6056,6 +6061,41 @@ void tet_adest_all_rcvr_thread()
free(mesg);
}
+uint32_t tet_mds_retrieve_for_change(MDS_HDL mds_hdl, MDS_SVC_ID
your_scv_id,
+ SaDispatchFlagsT dispatchFlags, NCSMDS_SVC_ID
req_svc_id,
+ MDS_SVC_PVT_SUB_PART_VER svc_pvt_ver, NCSMDS_CHG
change)
+{
+ unsigned int num_tries = 1;
+ unsigned int max_tries = 30;
+ uint32_t rc = NCSCC_RC_FAILURE;
+
+ while (1) {
+ if (mds_service_retrieve(mds_hdl, your_scv_id,
+ dispatchFlags) != NCSCC_RC_SUCCESS) {
+ printf("\nFail mds_service_retrieve\n");
+ break;
+ }
+ // Verify if the correct service event was retrieved.
+ if (tet_verify_version(mds_hdl, your_scv_id, req_svc_id,
+ svc_pvt_ver, change) == 1) {
+ rc = NCSCC_RC_SUCCESS;
+ break;
+ }
+ if (num_tries > max_tries) {
+ printf("\nCould not retrieve the expected event\n");
+ break;
+ }
+ else {
+ printf("\nExpected event was still not retrieved,"
+ " try again\n");
+ usleep(100000);
+ num_tries++;
+ }
+ }
+
+ return rc;
+}
+
void tet_send_all_tp_1()
{
int FAIL = 0;
@@ -6093,12 +6133,14 @@ void tet_send_all_tp_1()
FAIL = 1;
}
- if (mds_service_retrieve(gl_tet_adest.mds_pwe1_hdl,
- NCSMDS_SVC_ID_EXTERNAL_MIN,
- SA_DISPATCH_ALL) != NCSCC_RC_SUCCESS)
{
+ if (tet_mds_retrieve_for_change(gl_tet_adest.mds_pwe1_hdl,
+ NCSMDS_SVC_ID_EXTERNAL_MIN,
SA_DISPATCH_ALL,
+ NCSMDS_SVC_ID_EXTERNAL_MIN, 1,
+ NCSMDS_NO_ACTIVE) != NCSCC_RC_SUCCESS) {
printf("\nFail\n");
FAIL = 1;
}
+
/*SND*/
printf("\t Sending the message to no active instance\n");
if ((mds_just_send(
@@ -6135,9 +6177,11 @@ void tet_send_all_tp_1()
printf("\nFail\n");
FAIL = 1;
}
- if (mds_service_retrieve(gl_tet_adest.mds_pwe1_hdl,
- NCSMDS_SVC_ID_EXTERNAL_MIN,
- SA_DISPATCH_ALL) != NCSCC_RC_SUCCESS)
{
+
+ if (tet_mds_retrieve_for_change(gl_tet_adest.mds_pwe1_hdl,
+ NCSMDS_SVC_ID_EXTERNAL_MIN,
SA_DISPATCH_ALL,
+ NCSMDS_SVC_ID_EXTERNAL_MIN, 1,
+ NCSMDS_NEW_ACTIVE) != NCSCC_RC_SUCCESS) {
printf("\nFail\n");
FAIL = 1;
}
@@ -6261,12 +6305,14 @@ void tet_send_all_tp_2()
FAIL = 1;
}
- if (mds_service_retrieve(gl_tet_adest.mds_pwe1_hdl,
- NCSMDS_SVC_ID_EXTERNAL_MIN,
- SA_DISPATCH_ALL) != NCSCC_RC_SUCCESS)
{
+ if (tet_mds_retrieve_for_change(gl_tet_adest.mds_pwe1_hdl,
+ NCSMDS_SVC_ID_EXTERNAL_MIN,
SA_DISPATCH_ALL,
+ NCSMDS_SVC_ID_EXTERNAL_MIN, 1,
+ NCSMDS_NO_ACTIVE) != NCSCC_RC_SUCCESS) {
printf("\nFail\n");
FAIL = 1;
}
+
printf("\n Sending the message to no active instance\n");
if ((mds_just_send(gl_tet_adest.mds_pwe1_hdl,
NCSMDS_SVC_ID_EXTERNAL_MIN,
@@ -6282,25 +6328,29 @@ void tet_send_all_tp_2()
printf("\nFail\n");
FAIL = 1;
}
- if (mds_service_retrieve(gl_tet_adest.mds_pwe1_hdl,
- NCSMDS_SVC_ID_EXTERNAL_MIN,
- SA_DISPATCH_ALL) != NCSCC_RC_SUCCESS)
{
- printf("\nFail mds_service_retrieve\n");
+
+ if (tet_mds_retrieve_for_change(gl_tet_adest.mds_pwe1_hdl,
+ NCSMDS_SVC_ID_EXTERNAL_MIN,
SA_DISPATCH_ALL,
+ NCSMDS_SVC_ID_EXTERNAL_MIN, 1,
+ NCSMDS_NEW_ACTIVE) != NCSCC_RC_SUCCESS) {
+ printf("\nFail\n");
FAIL = 1;
}
+
/*SNDACK*/
printf("\nChange role to standby\n");
if (vdest_change_role(200, V_DEST_RL_STANDBY) !=
NCSCC_RC_SUCCESS) {
printf("\nFail\n");
FAIL = 1;
}
- if (mds_service_retrieve(gl_tet_adest.mds_pwe1_hdl,
- NCSMDS_SVC_ID_EXTERNAL_MIN,
- SA_DISPATCH_ALL) != NCSCC_RC_SUCCESS)
{
+
+ if (tet_mds_retrieve_for_change(gl_tet_adest.mds_pwe1_hdl,
+ NCSMDS_SVC_ID_EXTERNAL_MIN,
SA_DISPATCH_ALL,
+ NCSMDS_SVC_ID_EXTERNAL_MIN, 1,
+ NCSMDS_NO_ACTIVE) != NCSCC_RC_SUCCESS) {
printf("\nFail\n");
FAIL = 1;
}
-
else
printf("\nSuccess\n");
/*Create thread to change role*/
@@ -6337,15 +6387,16 @@ void tet_send_all_tp_2()
printf("\nFail\n");
FAIL = 1;
}
- if (mds_service_retrieve(gl_tet_adest.mds_pwe1_hdl,
- NCSMDS_SVC_ID_EXTERNAL_MIN,
- SA_DISPATCH_ALL) != NCSCC_RC_SUCCESS)
{
+ if (tet_mds_retrieve_for_change(gl_tet_adest.mds_pwe1_hdl,
+ NCSMDS_SVC_ID_EXTERNAL_MIN,
SA_DISPATCH_ALL,
+ NCSMDS_SVC_ID_EXTERNAL_MIN, 1,
+ NCSMDS_NO_ACTIVE) != NCSCC_RC_SUCCESS) {
printf("\nFail\n");
FAIL = 1;
}
-
else
printf("\nSuccess\n");
+
if (tet_create_task((NCS_OS_CB)tet_vdest_all_rcvr_thread,
gl_tet_vdest[1].svc[1].task.t_handle) ==
NCSCC_RC_SUCCESS) {
@@ -8977,12 +9028,14 @@ void tet_direct_send_all_tp_5()
FAIL = 1;
}
- if (mds_service_retrieve(gl_tet_adest.mds_pwe1_hdl,
- NCSMDS_SVC_ID_EXTERNAL_MIN,
- SA_DISPATCH_ALL) !=
NCSCC_RC_SUCCESS) {
+ if
(tet_mds_retrieve_for_change(gl_tet_adest.mds_pwe1_hdl,
+ NCSMDS_SVC_ID_EXTERNAL_MIN,
SA_DISPATCH_ALL,
+ NCSMDS_SVC_ID_EXTERNAL_MIN, 1,
+ NCSMDS_NO_ACTIVE) !=
NCSCC_RC_SUCCESS) {
printf("\nFail\n");
FAIL = 1;
}
+
printf("\n Sending the message to no active instance\n");
if (mds_direct_send_message(
gl_tet_adest.mds_pwe1_hdl,
NCSMDS_SVC_ID_EXTERNAL_MIN,
@@ -8999,12 +9052,15 @@ void tet_direct_send_all_tp_5()
printf("\nFail\n");
FAIL = 1;
}
- if (mds_service_retrieve(gl_tet_adest.mds_pwe1_hdl,
- NCSMDS_SVC_ID_EXTERNAL_MIN,
- SA_DISPATCH_ALL) !=
NCSCC_RC_SUCCESS) {
+
+ if
(tet_mds_retrieve_for_change(gl_tet_adest.mds_pwe1_hdl,
+ NCSMDS_SVC_ID_EXTERNAL_MIN,
SA_DISPATCH_ALL,
+ NCSMDS_SVC_ID_EXTERNAL_MIN, 1,
+ NCSMDS_NEW_ACTIVE) !=
NCSCC_RC_SUCCESS) {
printf("\nFail\n");
FAIL = 1;
}
+
/*SNDACK*/
printf("\nChange role to standby\n");
if (vdest_change_role(200, V_DEST_RL_STANDBY) !=
@@ -9012,9 +9068,11 @@ void tet_direct_send_all_tp_5()
printf("\nFail\n");
FAIL = 1;
}
- if (mds_service_retrieve(gl_tet_adest.mds_pwe1_hdl,
- NCSMDS_SVC_ID_EXTERNAL_MIN,
- SA_DISPATCH_ALL) !=
NCSCC_RC_SUCCESS) {
+
+ if
(tet_mds_retrieve_for_change(gl_tet_adest.mds_pwe1_hdl,
+ NCSMDS_SVC_ID_EXTERNAL_MIN,
SA_DISPATCH_ALL,
+ NCSMDS_SVC_ID_EXTERNAL_MIN, 1,
+ NCSMDS_NO_ACTIVE) !=
NCSCC_RC_SUCCESS) {
printf("\nFail\n");
FAIL = 1;
}
@@ -9057,13 +9115,14 @@ void tet_direct_send_all_tp_5()
printf("\nFail\n");
FAIL = 1;
}
- if (mds_service_retrieve(gl_tet_adest.mds_pwe1_hdl,
- NCSMDS_SVC_ID_EXTERNAL_MIN,
- SA_DISPATCH_ALL) !=
NCSCC_RC_SUCCESS) {
+
+ if
(tet_mds_retrieve_for_change(gl_tet_adest.mds_pwe1_hdl,
+ NCSMDS_SVC_ID_EXTERNAL_MIN,
SA_DISPATCH_ALL,
+ NCSMDS_SVC_ID_EXTERNAL_MIN, 1,
+ NCSMDS_NO_ACTIVE) !=
NCSCC_RC_SUCCESS) {
printf("\nFail\n");
FAIL = 1;
}
-
else
printf("\nSuccess\n");
@@ -9255,9 +9314,10 @@ void tet_direct_send_all_tp_6()
FAIL = 1;
}
- if (mds_service_retrieve(gl_tet_adest.mds_pwe1_hdl,
- NCSMDS_SVC_ID_EXTERNAL_MIN,
- SA_DISPATCH_ALL) !=
NCSCC_RC_SUCCESS) {
+ if
(tet_mds_retrieve_for_change(gl_tet_adest.mds_pwe1_hdl,
+ NCSMDS_SVC_ID_EXTERNAL_MIN,
SA_DISPATCH_ALL,
+ NCSMDS_SVC_ID_EXTERNAL_MIN, 1,
+ NCSMDS_NO_ACTIVE) !=
NCSCC_RC_SUCCESS) {
printf("\nFail\n");
FAIL = 1;
}
@@ -9313,9 +9373,11 @@ void tet_direct_send_all_tp_6()
printf("\nFail\n");
FAIL = 1;
}
- if (mds_service_retrieve(gl_tet_adest.mds_pwe1_hdl,
- NCSMDS_SVC_ID_EXTERNAL_MIN,
- SA_DISPATCH_ALL) !=
NCSCC_RC_SUCCESS) {
+
+ if
(tet_mds_retrieve_for_change(gl_tet_adest.mds_pwe1_hdl,
+ NCSMDS_SVC_ID_EXTERNAL_MIN,
SA_DISPATCH_ALL,
+ NCSMDS_SVC_ID_EXTERNAL_MIN, 1,
+ NCSMDS_NEW_ACTIVE) !=
NCSCC_RC_SUCCESS) {
printf("\nFail\n");
FAIL = 1;
}
--
2.7.4
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Opensaf-devel mailing list
[email protected]<mailto:[email protected]>
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
