[devel] [PATCH 1/2] mdstest: identify svcs using svc_id and mds_dest when storing event info [#2798]

2018-08-22 Thread Hoa Le
Currently, when updating the last event info, mdstest identify services
using their svc_id. This will cause confusion when several services was
installed with the same svc_id (on different mds_dest-s). If a service
subscribes to this svc_id, the service will retrieve several event info
with the same svc_id. When storing these event info to svcevt array, the
info are overwritten one by one and only the last info will be stored.

This patch helps avoid the above situation by identifying these
services using both their svc_id and mds_dest. This helps the event
info, from different service, be separatedly stored to svcevt array.
subscr_count value will also be updated in accordance with these
event info.
---
 src/mds/apitest/mdstipc_api.c  |  13 ++--
 src/mds/apitest/mdstipc_conf.c | 162 +
 2 files changed, 152 insertions(+), 23 deletions(-)

diff --git a/src/mds/apitest/mdstipc_api.c b/src/mds/apitest/mdstipc_api.c
index 22a4386..090af99 100644
--- a/src/mds/apitest/mdstipc_api.c
+++ b/src/mds/apitest/mdstipc_api.c
@@ -1090,7 +1090,7 @@ static int tet_verify_version(MDS_HDL mds_hdl, 
NCSMDS_SVC_ID your_scv_id,
  MDS_SVC_PVT_SUB_PART_VER svc_pvt_ver,
  NCSMDS_CHG change)
 {
-   int i, j, k, ret_val = 0;
+   int i, j, k;
if (is_service_on_adest(mds_hdl, your_scv_id) == NCSCC_RC_SUCCESS) {
for (i = 0; i < gl_tet_adest.svc_count; i++) {
if (gl_tet_adest.svc[i].svc_id == your_scv_id) {
@@ -1107,9 +1107,8 @@ static int tet_verify_version(MDS_HDL mds_hdl, 
NCSMDS_SVC_ID your_scv_id,
 .svcevt[j]
 .rem_svc_pvt_ver ==
 svc_pvt_ver))
-   ret_val = 1;
-   else
-   ret_val = 0;
+   return 1;
+
}
}
}
@@ -1137,16 +1136,14 @@ static int tet_verify_version(MDS_HDL mds_hdl, 
NCSMDS_SVC_ID your_scv_id,
 .svcevt[k]
 
.rem_svc_pvt_ver ==
 svc_pvt_ver))
-   ret_val = 1;
-   else
-   ret_val = 0;
+   return 1;
}
}
}
}
}
}
-   return ret_val;
+   return 0;
 }
 void tet_svc_subscr_VDEST_1()
 {
diff --git a/src/mds/apitest/mdstipc_conf.c b/src/mds/apitest/mdstipc_conf.c
index bf4c1de..61bf27f 100644
--- a/src/mds/apitest/mdstipc_conf.c
+++ b/src/mds/apitest/mdstipc_conf.c
@@ -557,7 +557,7 @@ uint32_t mds_service_subscribe(MDS_HDL mds_hdl, MDS_SVC_ID 
svc_id,
   NCSMDS_SCOPE_TYPE scope, uint8_t num_svcs,
   MDS_SVC_ID *svc_ids)
 {
-   int i, j, k, l, FOUND;
+   int i, j, l, FOUND;
uint32_t YES_ADEST;
NCSMDS_INFO svc_to_mds_info;
/*Find whether this Service is on Adest or Vdest*/
@@ -594,6 +594,7 @@ uint32_t mds_service_subscribe(MDS_HDL mds_hdl, MDS_SVC_ID 
svc_id,
.svc[j]
.subscr.svc_ids = svc_ids;

/*gl_tet_vdest[i].svc[j].subscr.evt_map=*/
+   /* Fix for ticket #2798
gl_tet_vdest[i]
.svc[j]
.subscr_count += num_svcs;
@@ -611,6 +612,7 @@ uint32_t mds_service_subscribe(MDS_HDL mds_hdl, MDS_SVC_ID 
svc_id,
.svcevt[k]
.ur_svc_id = svc_id;
}
+   */
FOUND = 1;
break;
}
@@ -642,6 +644,7 @@ uint32_t mds_service_subscribe(MDS_HDL mds_hdl, MDS_SVC_ID 
svc_id,
.svc_ids =
 

[devel] [PATCH 0/2] Review Request for mdstest: correct timing issues in mdstest V3 [#2798]

2018-08-22 Thread Hoa Le
Summary: mdstest: identify svcs using svc_id and mds_dest when storing event 
info [#2798]
Review request for Ticket(s): 2798
Peer Reviewer(s): Hans, Lennart, Minh
Pull request to: Hans, Lennart, Minh
Affected branch(es): develop
Development branch: ticket-2798
Base revision: 998c5b0f0aa14900c9408bc703ea3c2e99b854e8
Personal repository: git://git.code.sf.net/u/xhoalee/review


Impacted area   Impact y/n

 Docsn
 Build systemn
 RPM/packaging   n
 Configuration files n
 Startup scripts n
 SAF servicesn
 OpenSAF servicesn
 Core libraries  n
 Samples n
 Tests   y
 Other   n

NOTE: Patch(es) contain lines longer than 80 characers

Comments (indicate scope for each "y" above):
-


revision c8ef4483ece445e281cb7bdeac3879cfd0a2c878
Author: Hoa Le 
Date:   Thu, 23 Aug 2018 09:21:09 +0700

mdstest: correct timing issues in mdstest [#2798]

In some bad thread scheduling situations, the API service request
in the testing thread may be executed before the corresponding
event being received on the MDS thread. This will lead to the
unexpected behavior of the service request and cause the failure
in this test case.

This patch helps avoid the above issue by waiting for the expected
event being received on MDS thread before invoking the testing
service request.



revision a67972060346225931e8c9fd89589ea9bc52e332
Author: Hoa Le 
Date:   Thu, 23 Aug 2018 09:21:09 +0700

mdstest: identify svcs using svc_id and mds_dest when storing event info [#2798]

Currently, when updating the last event info, mdstest identify services
using their svc_id. This will cause confusion when several services was
installed with the same svc_id (on different mds_dest-s). If a service
subscribes to this svc_id, the service will retrieve several event info
with the same svc_id. When storing these event info to svcevt array, the
info are overwritten one by one and only the last info will be stored.

This patch helps avoid the above situation by identifying these
services using both their svc_id and mds_dest. This helps the event
info, from different service, be separatedly stored to svcevt array.
subscr_count value will also be updated in accordance with these
event info.



Complete diffstat:
--
 src/mds/apitest/mdstipc_api.c  | 410 +++--
 src/mds/apitest/mdstipc_conf.c | 164 +++--
 2 files changed, 421 insertions(+), 153 deletions(-)


Testing Commands:
-
mdstest 4 10
mdstest 4 12
mdstest 5 1
mdstest 5 9
mdstest 10 1
mdstest 10 2
mdstest 14 5
mdstest 14 6


Testing, Expected Results:
--
No failure appears.


Conditions of Submission:
-
ACK from reviewer.


Arch  Built StartedLinux distro
---
mipsn  n
mips64  n  n
x86 n  n
x86_64  y  y
powerpc n  n
powerpc64   n  n


Reviewer Checklist:
---
[Submitters: make sure that your review doesn't trigger any checkmarks!]


Your checkin has not passed review because (see checked entries):

___ Your RR template is generally incomplete; it has too many blank entries
that need proper data filled in.

___ You have failed to nominate the proper persons for review and push.

___ Your patches do not have proper short+long header

___ You have grammar/spelling in your header that is unacceptable.

___ You have exceeded a sensible line length in your headers/comments/text.

___ You have failed to put in a proper Trac Ticket # into your commits.

___ You have incorrectly put/left internal data in your comments/files
(i.e. internal bug tracking tool IDs, product names etc)

___ You have not given any evidence of testing beyond basic build tests.
Demonstrate some level of runtime or other sanity testing.

___ You have ^M present in some of your files. These have to be removed.

___ You have needlessly changed whitespace or added whitespace crimes
like trailing spaces, or spaces before tabs.

___ You have mixed real technical changes with whitespace and other
cosmetic code cleanup changes. These have to be separate commits.

___ You need to refactor your submission into logical chunks; there is
too much content into a single commit.

___ You have extraneous garbage in your review (merge commits etc)

___ You have giant attachments which should never have been sent;
Instead you should place your content in a public tree to be pulled.

___ You have too many commits attached to an e-mail; resend as threaded
commits, or place in a public tree for a pull.

___ You have resent this content multiple times without a clear indication
of what has changed between each re-send.

___ You have 

[devel] [PATCH 2/2] mdstest: correct timing issues in mdstest [#2798]

2018-08-22 Thread Hoa Le
In some bad thread scheduling situations, the API service request
in the testing thread may be executed before the corresponding
event being received on the MDS thread. This will lead to the
unexpected behavior of the service request and cause the failure
in this test case.

This patch helps avoid the above issue by waiting for the expected
event being received on MDS thread before invoking the testing
service request.
---
 src/mds/apitest/mdstipc_api.c  | 397 -
 src/mds/apitest/mdstipc_conf.c |   2 -
 2 files changed, 269 insertions(+), 130 deletions(-)

diff --git a/src/mds/apitest/mdstipc_api.c b/src/mds/apitest/mdstipc_api.c
index 090af99..2450f22 100644
--- a/src/mds/apitest/mdstipc_api.c
+++ b/src/mds/apitest/mdstipc_api.c
@@ -27,11 +27,13 @@
 #include "base/ncs_main_papi.h"
 #include "mdstipc.h"
 #include "base/ncssysf_tmr.h"
+#include "base/osaf_time.h"
 
 #define MSG_SIZE MDS_DIRECT_BUF_MAXSIZE
 static MDS_CLIENT_MSG_FORMAT_VER gl_set_msg_fmt_ver;
 
 MDS_SVC_ID svc_ids[3] = {2006, 2007, 2008};
+const uint64_t kWait10sec = 10*1000;
 
 pthread_mutex_t safe_printf_mutex = PTHREAD_MUTEX_INITIALIZER;
 pthread_mutex_t gl_mutex = PTHREAD_MUTEX_INITIALIZER;
@@ -1652,14 +1654,72 @@ void tet_svc_subscr_VDEST_7()
test_validate(FAIL, 0);
 }
 
+
+/*
+
+  Function NAME: tet_mds_retrieve_for_change
+
+  DESCRIPTION: This routine calls mds_service_retrieve function until
+  successfully retrieving the given service subscription state change event.
+  It returns NCSCC_RC_FAILURE if the expected event cannot be retrieved within
+  predefined kWait10sec timeout period or when the mds_service_retrieve call
+  results in NCSCC_RC_FAILURE.
+
+  ARGUMENTS:
+mds_hdl:   MDS_HDL related to the subscribing service.
+your_scv_id:   MDS_SVC_ID of the subscribing service.
+dispatchFlags: SaDispatchFlagsT
+req_svc_id:NCSMDS_SVC_ID of the subscribed service.
+svc_pvt_ver:   MDS svc private sub part version of the subscribed service.
+change:Expected NCSMDS_CHG of the subscribed service.
+
+  RETURNS:  1 - NCSCC_RC_SUCCESS
+2 - NCSCC_RC_FAILURE
+
+*/
+
+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)
+{
+   uint32_t rc = NCSCC_RC_FAILURE;
+   struct timespec wait_time;
+
+   osaf_set_millis_timeout(kWait10sec, _time);
+   while (!osaf_is_timeout(_time)) {
+   osaf_nanosleep();
+   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;
+   }
+   }
+   // The expected event was not retrieved within reasonable time.
+   if (osaf_is_timeout(_time) && rc != NCSCC_RC_SUCCESS) {
+   printf("\nFail to retrieve the expected event\n");
+   }
+
+   return rc;
+}
+
+
 void tet_svc_subscr_VDEST_10()
 {
bool FAIL = false;
SaUint32T rc;
int id;
MDS_SVC_ID svcids[] = {600, 700};
-   printf(
-   "\nTest Case 10: Cross check whether i_rem_svc_pvt_ver returned in 
service event callback, matches the service private sub-part version of the 
remote service (subscribee)\n");
+
+   printf("\nTest Case 10: Cross check whether i_rem_svc_pvt_ver returned "
+   "in service event callback, matches the service "
+   "private sub-part version of the remote service "
+   "(subscribed)\n");
// Creating a N-way VDEST with id =1001
rc = create_vdest(NCS_VDEST_TYPE_N_WAY_ROUND_ROBIN, 1001);
if (rc != NCSCC_RC_SUCCESS) {
@@ -1710,40 +1770,44 @@ void tet_svc_subscr_VDEST_10()
printf("\nFail to change role\n");
FAIL = 1;
}
-   // Retrieving the events
-   if (mds_service_retrieve(gl_tet_adest.mds_pwe1_hdl, 500,
-SA_DISPATCH_ALL) != NCSCC_RC_SUCCESS) {
-   printf("\nRetrieve servive 500 Fail\n");
+   // Wait for NCSMDS_UP event of service 600
+   if (tet_mds_retrieve_for_change(gl_tet_adest.mds_pwe1_hdl,
+   500, SA_DISPATCH_ALL, 600, 1, NCSMDS_UP) != 
NCSCC_RC_SUCCESS) {
+   printf("\nFail to retrieve NCSMDS_UP of service id 600\n");
FAIL 

Re: [devel] [PATCH 1/1] amfd: Set SA_AMF_READINESS_IN_SERVICE for qualified SU after cluster startup timeout [#2916]

2018-08-22 Thread Gary Lee

Hi Minh

ack (review)

Thanks Gary

On 22/08/18 20:23, Minh Chau wrote:

In the scenario of single step upgrade where the UNLOCK-IN/UNLOCK
admin op are issued to a SU hosted on non-active node while cluster
startup timer is active and not all ncs SU on that node are fully
assigned. In such case, amfd currently accepts the UNLOCK admin op,
change AdminState to UNLOCKED but the ReadinessState is still OUT_OF_SERVICE.
When the cluster startup timeout, amfd is not giving assignment to
this SU which is still OUT_OF_SERVICE. amfd should return TRY_AGAIN
in the first place of UNLOCK command since the node is still DISABLED
and ABSENT since all ncs SUs are not assigned, but it would cause
the upgrade delays.

When cluster startup timer expires, the patch checks if SU is qualified
to be IN_SERVICE but its readiness state is still OUT_OF_SERVICE, set
it to be IN_SERVICE before amfd starts assignments.
---
  src/amf/amfd/cluster.cc | 13 +
  1 file changed, 13 insertions(+)

diff --git a/src/amf/amfd/cluster.cc b/src/amf/amfd/cluster.cc
index 156c5c9..83fd47d 100644
--- a/src/amf/amfd/cluster.cc
+++ b/src/amf/amfd/cluster.cc
@@ -85,6 +85,19 @@ void avd_cluster_tmr_init_evh(AVD_CL_CB *cb, AVD_EVT *evt) {
  node->node_info.nodeId != cb->node_id_avd_other)
avd_snd_set_leds_msg(cb, node);
}
+  /* If the SU is qualified to be IN_SERVICE but its readiness state
+   * is still OUT_OF_SERVICE, set it to be IN_SERVICE.
+   * This scenario happens if the UNLOCK-in/UNLOCK admin op is issued
+   * to a SU hosted on non-active node while cluster startup timer is
+   * active and not all ncs SU on that node are fully assigned
+   */
+  for (const auto  : *su_db) {
+AVD_SU *i_su = value.second;
+if (i_su->is_in_service() && i_su->sg_of_su->sg_ncs_spec == false &&
+i_su->saAmfSuReadinessState == SA_AMF_READINESS_OUT_OF_SERVICE) {
+  i_su->set_readiness_state(SA_AMF_READINESS_IN_SERVICE);
+}
+  }
  
/* call the realignment routine for each of the SGs in the

 * system that are not NCS specific.



--
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
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel


Re: [devel] [PATCH 1/1] amfd: Set SA_AMF_READINESS_IN_SERVICE for qualified SU after cluster startup timeout [#2916]

2018-08-22 Thread Hans Nordeback

ack, review only/Thanks HansN


On 08/22/2018 12:23 PM, Minh Chau wrote:

In the scenario of single step upgrade where the UNLOCK-IN/UNLOCK
admin op are issued to a SU hosted on non-active node while cluster
startup timer is active and not all ncs SU on that node are fully
assigned. In such case, amfd currently accepts the UNLOCK admin op,
change AdminState to UNLOCKED but the ReadinessState is still OUT_OF_SERVICE.
When the cluster startup timeout, amfd is not giving assignment to
this SU which is still OUT_OF_SERVICE. amfd should return TRY_AGAIN
in the first place of UNLOCK command since the node is still DISABLED
and ABSENT since all ncs SUs are not assigned, but it would cause
the upgrade delays.

When cluster startup timer expires, the patch checks if SU is qualified
to be IN_SERVICE but its readiness state is still OUT_OF_SERVICE, set
it to be IN_SERVICE before amfd starts assignments.
---
  src/amf/amfd/cluster.cc | 13 +
  1 file changed, 13 insertions(+)

diff --git a/src/amf/amfd/cluster.cc b/src/amf/amfd/cluster.cc
index 156c5c9..83fd47d 100644
--- a/src/amf/amfd/cluster.cc
+++ b/src/amf/amfd/cluster.cc
@@ -85,6 +85,19 @@ void avd_cluster_tmr_init_evh(AVD_CL_CB *cb, AVD_EVT *evt) {
  node->node_info.nodeId != cb->node_id_avd_other)
avd_snd_set_leds_msg(cb, node);
}
+  /* If the SU is qualified to be IN_SERVICE but its readiness state
+   * is still OUT_OF_SERVICE, set it to be IN_SERVICE.
+   * This scenario happens if the UNLOCK-in/UNLOCK admin op is issued
+   * to a SU hosted on non-active node while cluster startup timer is
+   * active and not all ncs SU on that node are fully assigned
+   */
+  for (const auto  : *su_db) {
+AVD_SU *i_su = value.second;
+if (i_su->is_in_service() && i_su->sg_of_su->sg_ncs_spec == false &&
+i_su->saAmfSuReadinessState == SA_AMF_READINESS_OUT_OF_SERVICE) {
+  i_su->set_readiness_state(SA_AMF_READINESS_IN_SERVICE);
+}
+  }
  
/* call the realignment routine for each of the SGs in the

 * system that are not NCS specific.



--
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
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel


[devel] [PATCH 1/1] amfd: Set SA_AMF_READINESS_IN_SERVICE for qualified SU after cluster startup timeout [#2916]

2018-08-22 Thread Minh Chau
In the scenario of single step upgrade where the UNLOCK-IN/UNLOCK
admin op are issued to a SU hosted on non-active node while cluster
startup timer is active and not all ncs SU on that node are fully
assigned. In such case, amfd currently accepts the UNLOCK admin op,
change AdminState to UNLOCKED but the ReadinessState is still OUT_OF_SERVICE.
When the cluster startup timeout, amfd is not giving assignment to
this SU which is still OUT_OF_SERVICE. amfd should return TRY_AGAIN
in the first place of UNLOCK command since the node is still DISABLED
and ABSENT since all ncs SUs are not assigned, but it would cause
the upgrade delays.

When cluster startup timer expires, the patch checks if SU is qualified
to be IN_SERVICE but its readiness state is still OUT_OF_SERVICE, set
it to be IN_SERVICE before amfd starts assignments.
---
 src/amf/amfd/cluster.cc | 13 +
 1 file changed, 13 insertions(+)

diff --git a/src/amf/amfd/cluster.cc b/src/amf/amfd/cluster.cc
index 156c5c9..83fd47d 100644
--- a/src/amf/amfd/cluster.cc
+++ b/src/amf/amfd/cluster.cc
@@ -85,6 +85,19 @@ void avd_cluster_tmr_init_evh(AVD_CL_CB *cb, AVD_EVT *evt) {
 node->node_info.nodeId != cb->node_id_avd_other)
   avd_snd_set_leds_msg(cb, node);
   }
+  /* If the SU is qualified to be IN_SERVICE but its readiness state
+   * is still OUT_OF_SERVICE, set it to be IN_SERVICE.
+   * This scenario happens if the UNLOCK-in/UNLOCK admin op is issued
+   * to a SU hosted on non-active node while cluster startup timer is
+   * active and not all ncs SU on that node are fully assigned
+   */
+  for (const auto  : *su_db) {
+AVD_SU *i_su = value.second;
+if (i_su->is_in_service() && i_su->sg_of_su->sg_ncs_spec == false &&
+i_su->saAmfSuReadinessState == SA_AMF_READINESS_OUT_OF_SERVICE) {
+  i_su->set_readiness_state(SA_AMF_READINESS_IN_SERVICE);
+}
+  }
 
   /* call the realignment routine for each of the SGs in the
* system that are not NCS specific.
-- 
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
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel


Re: [devel] [PATCH 1/1] log: add new test case of API saLogInitialize() of apitest [#2915]

2018-08-22 Thread Vu Minh Nguyen
Ack.

Thanks, Vu

> -Original Message-
> From: ratnamala 
> Sent: Tuesday, August 21, 2018 8:46 PM
> To: lennart.l...@ericsson.com; vu.m.ngu...@dektech.com.au
> Cc: opensaf-devel@lists.sourceforge.net; ratnamala
> 
> Subject: [PATCH 1/1] log: add new test case of API saLogInitialize() of
apitest
> [#2915]
> 
> ---
>  src/log/apitest/tet_saLogInitialize.c | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/src/log/apitest/tet_saLogInitialize.c
> b/src/log/apitest/tet_saLogInitialize.c
> index 3d31c8e..68c965d 100644
> --- a/src/log/apitest/tet_saLogInitialize.c
> +++ b/src/log/apitest/tet_saLogInitialize.c
> @@ -108,6 +108,12 @@ void saLogInitialize_11(void)
>   test_validate(rc, SA_AIS_OK);
>  }
> 
> +void saLogInitialize_12(void)
> +{
> + rc = saLogInitialize(, , NULL);
> + logFinalize();
> + test_validate(rc, SA_AIS_ERR_INVALID_PARAM);
> +}
>  extern void saLogSelectionObjectGet_01(void);
>  extern void saLogSelectionObjectGet_02(void);
>  extern void saLogFinalize_01(void);
> @@ -139,6 +145,8 @@ __attribute__((constructor)) static void
> saLibraryLifeCycle_constructor(void)
>   "saLogInitialize() with minor version is set bigger than
supported
> version");
>   test_case_add(1, saLogInitialize_11,
> "saLogInitialize() with minor version is not set");
> + test_case_add(1, saLogInitialize_12,
> + "saLogInitialize() with valid handle,valid callbacks and
version
> as NULL");
>   test_case_add(1, saLogSelectionObjectGet_01,
> "saLogSelectionObjectGet() OK");
>   test_case_add(1, saLogSelectionObjectGet_02,
> --
> 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
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel


Re: [devel] [PATCH 1/1] log: add new test case of API saLogInitialize() of apitest [#2915]

2018-08-22 Thread Lennart Lund
Hi,

Ack. See my comments below tagged [Lennart]

Thanks
Lennart

> -Original Message-
> From: ratnamala 
> Sent: den 21 augusti 2018 15:46
> To: Lennart Lund ; Vu Minh Nguyen
> 
> Cc: opensaf-devel@lists.sourceforge.net; ratnamala
> 
> Subject: [PATCH 1/1] log: add new test case of API saLogInitialize() of 
> apitest
> [#2915]
> 
> ---
>  src/log/apitest/tet_saLogInitialize.c | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/src/log/apitest/tet_saLogInitialize.c
> b/src/log/apitest/tet_saLogInitialize.c
> index 3d31c8e..68c965d 100644
> --- a/src/log/apitest/tet_saLogInitialize.c
> +++ b/src/log/apitest/tet_saLogInitialize.c
> @@ -108,6 +108,12 @@ void saLogInitialize_11(void)
>   test_validate(rc, SA_AIS_OK);
>  }
> 
[Lennart] Add information about what's tested. All test cases should have a 
description telling WHAT is tested.
Note that a function like this where it is quite obvious what it is doing 
normally do not need a specific description but
test cases is an exception. You should also give the test case a descriptive 
name (will document the test case where it is called)
Unfortunately most test cases are poorly documented and any big effort to fix 
this may not be worth the effort but it should be done for new and updated test 
cases.
Suggest that you do something like this:
// Test that SA_AIS_ERR_INVALID_PARAM is returned if version parameter is set 
to a NULL pointer
> -void saLogInitialize_12(void)
+ void saLogInitialize_version_NULL_pointer_error(void)
> +{
> + rc = saLogInitialize(, , NULL);
> + logFinalize();
> + test_validate(rc, SA_AIS_ERR_INVALID_PARAM);
> +}
>  extern void saLogSelectionObjectGet_01(void);
>  extern void saLogSelectionObjectGet_02(void);
>  extern void saLogFinalize_01(void);
> @@ -139,6 +145,8 @@ __attribute__((constructor)) static void
> saLibraryLifeCycle_constructor(void)
>   "saLogInitialize() with minor version is set bigger than supported
> version");
>   test_case_add(1, saLogInitialize_11,
> "saLogInitialize() with minor version is not set");
> + test_case_add(1, saLogInitialize_12,
> + "saLogInitialize() with valid handle,valid callbacks and version
> as NULL");
>   test_case_add(1, saLogSelectionObjectGet_01,
> "saLogSelectionObjectGet() OK");
>   test_case_add(1, saLogSelectionObjectGet_02,
> --
> 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
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel