Re: [devel] [PATCH 1 of 1] amfd: Add support for google unit test framework V2 [#1142]
On 2015-05-26 07:23, praveen malviya wrote: On 08-May-15 1:41 PM, Hans Nordeback wrote: 00-README.unittest | 32 configure.ac| 1 + osaf/services/saf/amf/amfd/Makefile.am | 2 +- osaf/services/saf/amf/amfd/tests/Makefile.am| 50 + osaf/services/saf/amf/amfd/tests/test_amf_db.cc | 44 ++ 5 files changed, 128 insertions(+), 1 deletions(-) As part of refactoring enable the use of google unit test framework. diff --git a/00-README.unittest b/00-README.unittest new file mode 100644 --- /dev/null +++ b/00-README.unittest @@ -0,0 +1,32 @@ +Support for using google unit test in openSAF. Using unit test during e.g. refactoring +to identify units and make code unit testable should improve the overall code quality +and robustness. One questions, as I am not much familiar with google test framework: Besides testing the code units (refactored or newly developed), could this framework be used for functional testing for a distributed scenario? A AMF model is distributed across the nodes and a functional test case may require sync among nodes. Well you can but you probably/most likely want to write such framework in for example python and not C/C++. Thanks, Hans Thanks, Praveen +Regarding google unit test, see: +https://code.google.com/p/googletest/ + +To install google test in Ubuntu 14.04 do: +sudo apt-get install libgtest-dev +sudo apt-get install cmake + +cd /usr/src/gtest +sudo cmake . +sudo make +sudo mv libg* /usr/lib/ + +Add the following environment variable where the gtest header files are located: +export GTEST_DIR=/usr/include/gtest + +configure openSAF as usual, for example +./bootstrap.ch +./configure CFLAGS=-DRUNASROOT -O2 CXXFLAGS=-DRUNASROOT -O2 --enable-tipc + +make -j + +To build and run the unit tests +make check + +A unit test report will be written on the console. + +The unit test cases are to be placed in a new directory called tests at the same directory level as e.g. include, +scripts etc. +Unit test logs etc. can be found under the tests directory. \ No newline at end of file diff --git a/configure.ac b/configure.ac --- a/configure.ac +++ b/configure.ac @@ -801,6 +801,7 @@ AC_CONFIG_FILES([ osaf/services/saf/amf/amfd/include/Makefile osaf/services/saf/amf/amfd/scripts/Makefile osaf/services/saf/amf/amfd/scripts/osaf-amfd +osaf/services/saf/amf/amfd/tests/Makefile osaf/services/saf/amf/amfnd/Makefile osaf/services/saf/amf/amfnd/include/Makefile osaf/services/saf/amf/amfnd/scripts/Makefile diff --git a/osaf/services/saf/amf/amfd/Makefile.am b/osaf/services/saf/amf/amfd/Makefile.am --- a/osaf/services/saf/amf/amfd/Makefile.am +++ b/osaf/services/saf/amf/amfd/Makefile.am @@ -18,7 +18,7 @@ include $(top_srcdir)/Makefile.common MAINTAINERCLEANFILES = Makefile.in -SUBDIRS = include scripts +SUBDIRS = include scripts tests osaf_execbindir = $(pkglibdir) osaf_execbin_PROGRAMS = osafamfd diff --git a/osaf/services/saf/amf/amfd/tests/Makefile.am b/osaf/services/saf/amf/amfd/tests/Makefile.am new file mode 100644 --- /dev/null +++ b/osaf/services/saf/amf/amfd/tests/Makefile.am @@ -0,0 +1,50 @@ +# -*- OpenSAF -*- +# +# (C) Copyright 2015 The OpenSAF Foundation +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY +# or FITNESS FOR A PARTICULAR PURPOSE. This file and program are licensed +# under the GNU Lesser General Public License Version 2.1, February 1999. +# The complete license can be accessed from the following location: +# http://opensource.org/licenses/lgpl-license.php +# See the Copying file included with the OpenSAF distribution for full +# licensing terms. +# +# Author(s): Ericsson AB +# + +include $(top_srcdir)/Makefile.common + +TESTS = testamfd + +check_PROGRAMS = testamfd + +testamfd_CXXFLAGS =$(AM_CXXFLAGS) + +testamfd_CPPFLAGS = \ +-DSA_CLM_B01=1 \ +$(AM_CPPFLAGS) \ +-I$(top_srcdir)/osaf/libs/common/amf/include \ +-I$(top_srcdir)/osaf/services/saf/amf/amfd/include \ +-I$(top_srcdir)/osaf/libs/common/immsv/include \ +-I$(top_srcdir)/osaf/tools/saflog/include \ +-I$(GTEST_DIR) + +testamfd_LDFLAGS = \ +-lgtest -lpthread + +testamfd_SOURCES = \ +test_amf_db.cc + +testamfd_LDADD = \ +$(top_builddir)/osaf/tools/saflog/src/libsaflog.la \ +$(top_builddir)/osaf/tools/safimm/src/libimmutil.la \ +$(top_builddir)/osaf/libs/core/libopensaf_core.la \ +$(top_builddir)/osaf/libs/common/amf/libamf_common.la \ +$(top_builddir)/osaf/libs/saf/libSaNtf/libSaNtf.la \ +$(top_builddir)/osaf/libs/saf/libSaImm/libSaImmOi.la \ +
Re: [devel] [PATCH 0 of 1] Review Request for amfd: Add support for google unit test framework V2 [#1142]
Ack (for both patches on this ticket), with some comments: * Boiler plates (licence copyright) are missing in the new files * The README assumes the developer uses Ubuntu and has sudo rights (root access). Instead, I think it would be better to describe how to download the Google test framework from the web and build it. This would work on any Linux distribution and does not require root access. I.e. something like this: wget https://googletest.googlecode.com/files/gtest-1.7.0.zip unzip gtest-1.7.0.zip cd gtest-1.7.0 ./configure make export GTEST_DIR=`pwd` * Based on the instructions above, we also need to add -L$(GTEST_DIR)/lib to LDFLAGS in the Makefile, since libgtest.a is not installed in /usr/lib * When I tested this on Ubuntu 15.04 and gtest 1.7.0, I had to do some modifications to make it build successfully. The attached file shows what I had to change to make it build (including the -L$(GTEST_DIR)/lib mentioned above). / Anders W On 05/08/2015 10:11 AM, Hans Nordeback wrote: Summary: amfd: Add support for google unit test framework V2 Review request for Trac Ticket(s): #1142 Peer Reviewer(s): Praveen, Nagu, Mathi, AndersW Pull request to: Affected branch(es): default Development branch: default Impacted area Impact y/n Docsn Build systemn RPM/packaging n Configuration files n Startup scripts n SAF servicesy OpenSAF servicesn Core libraries n Samples n Tests n Other n Comments (indicate scope for each y above): - EXPLAIN/COMMENT THE PATCH SERIES HERE changeset 6be42cd2de89b3a8b8e282d35e996e947eedb564 Author: Hans Nordeback hans.nordeb...@ericsson.com Date: Fri, 08 May 2015 10:08:19 +0200 amfd: Add support for google unit test framework V2 [#1142] As part of refactoring enable the use of google unit test framework. Added Files: 00-README.unittest osaf/services/saf/amf/amfd/tests/Makefile.am osaf/services/saf/amf/amfd/tests/test_amf_db.cc Complete diffstat: -- 00-README.unittest | 32 configure.ac| 1 + osaf/services/saf/amf/amfd/Makefile.am | 2 +- osaf/services/saf/amf/amfd/tests/Makefile.am| 50 + osaf/services/saf/amf/amfd/tests/test_amf_db.cc | 44 ++ 5 files changed, 128 insertions(+), 1 deletions(-) Testing Commands: - LIST THE COMMAND LINE TOOLS/STEPS TO TEST YOUR CHANGES Testing, Expected Results: -- PASTE COMMAND OUTPUTS / TEST RESULTS Conditions of Submission: - HOW MANY DAYS BEFORE PUSHING, CONSENSUS ETC 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
[devel] [PATCH 0 of 1] Review Request for IMM: Detour of accessorGet into ImmModel::searchInitialize removed [#674]
Summary: IMM: Detour of accessorGet into ImmModel::searchInitialize removed [#674] Review request for Trac Ticket(s): 674 Peer Reviewer(s): Neel; Zoran Pull request to: Affected branch(es): default(4.7) Development branch: default(4.7) Impacted area Impact y/n Docsn Build systemn RPM/packaging n Configuration files n Startup scripts n SAF servicesy OpenSAF servicesn Core libraries n Samples n Tests n Other n Comments (indicate scope for each y above): - changeset ec4019e68ed3a24a738d9cc403fb7277e188f0d6 Author: Anders Bjornerstedt anders.bjornerst...@ericsson.com Date: Tue, 26 May 2015 14:08:00 +0200 IMM: Detour of accessorGet into ImmModel::searchInitialize removed [#674] The detour seemed mostly to deal with handling of the SaImmScopeT type. The scope of SA_IMM_ONE is only relevant for accessors and there it is implicit, since that function only deals with fetching attributes from one imm object. Complete diffstat: -- osaf/services/saf/immsv/immnd/ImmModel.cc | 30 +++--- osaf/services/saf/immsv/immnd/immnd_evt.c | 2 +- 2 files changed, 16 insertions(+), 16 deletions(-) Testing Commands: - This is a minor code cleanup. No change in behavior is expected. Testing, Expected Results: -- All/any regression test should pass. Conditions of Submission: - Ack from Neel. Arch Built StartedLinux distro --- mipsn n mips64 n n x86 n n x86_64 n n 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 failed to adequately and individually address all of the comments and change requests that were proposed in the initial review. ___ You have a misconfigured ~/.hgrc file (i.e. username, email etc) ___ Your computer have a badly configured date and time; confusing the the threaded patch review. ___ Your changes affect IPC mechanism, and you don't present any results for in-service upgradability test. ___ Your changes affect user manual and documentation, your patch series do not contain the patch that updates the Doxygen manual. -- One dashboard for servers and applications across Physical-Virtual-Cloud Widest out-of-the-box monitoring support with 50+ applications Performance metrics, stats and reports that give you Actionable Insights Deep dive visibility with transaction tracing using APM Insight. http://ad.doubleclick.net/ddm/clk/290420510;117567292;y ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
[devel] [PATCH 1 of 1] IMM: Detour of accessorGet into ImmModel::searchInitialize removed [#674]
osaf/services/saf/immsv/immnd/ImmModel.cc | 30 +++---
osaf/services/saf/immsv/immnd/immnd_evt.c | 2 +-
2 files changed, 16 insertions(+), 16 deletions(-)
The detour seemed mostly to deal with handling of the SaImmScopeT type.
The scope of SA_IMM_ONE is only relevant for accessors and there it is
implicit, since that function only deals with fetching attributes from
one imm object.
diff --git a/osaf/services/saf/immsv/immnd/ImmModel.cc
b/osaf/services/saf/immsv/immnd/ImmModel.cc
--- a/osaf/services/saf/immsv/immnd/ImmModel.cc
+++ b/osaf/services/saf/immsv/immnd/ImmModel.cc
@@ -1374,6 +1374,7 @@ SaAisErrorT
immModel_searchInitialize(IMMND_CB *cb, struct ImmsvOmSearchInit* req,
void** searchOp, SaBoolT isSync, SaBoolT isAccessor)
{
+SaAisErrorT err = SA_AIS_OK;
ImmSearchOp* op = new ImmSearchOp();
*searchOp = op;
@@ -1386,8 +1387,16 @@ immModel_searchInitialize(IMMND_CB *cb,
TRACE(Allocating iterator searchOp:%p, op);
}
-
-return ImmModel::instance(cb-immModel)-searchInitialize(req, *op);
+/* Reset search time */
+op-updateSearchTime();
+
+if(isAccessor) {
+err = ImmModel::instance(cb-immModel)-accessorGet(req, *op);
+} else {
+err = ImmModel::instance(cb-immModel)-searchInitialize(req, *op);
+}
+
+return err;
}
SaAisErrorT
@@ -10051,6 +10060,10 @@ ImmModel::accessorGet(const ImmsvOmSearc
SaImmSearchOptionsT notAllowedOptions = 0LL;
bool nonExtendedNameCheck = req-searchParam.present
ImmOmSearchParameter_PR_oneAttrParam;
bool checkAttribute = false;
+
+if(nonExtendedNameCheck) {
+op.setNonExtendedName();
+}
if (objectName.empty()) {
LOG_NO(ERR_INVALID_PARAM: Empty DN is not allowed);
@@ -10390,23 +10403,10 @@ ImmModel::searchInitialize(ImmsvOmSearch
std::string refObjectName;
SaUint32T childCount=0;
-/* Reset search time */
-op.updateSearchTime();
-
if(nonExtendedNameCheck) {
op.setNonExtendedName();
}
-if(scope == SA_IMM_ONE) {
-if(noDanglingSearch) {
- LOG_NO(ERR_INVALID_PARAM: SA_IMM_SEARCH_NO_DANGLING_DEPENDENTS
- flag cannot be used with SA_IMM_ONE scope);
- return SA_AIS_ERR_INVALID_PARAM;
-}
-
-return this-accessorGet(req, op);
-}
-
size_t sz = strnlen((char *) req-rootName.buf,
(size_t)req-rootName.size);
std::string rootName((const char*)req-rootName.buf, sz);
diff --git a/osaf/services/saf/immsv/immnd/immnd_evt.c
b/osaf/services/saf/immsv/immnd/immnd_evt.c
--- a/osaf/services/saf/immsv/immnd/immnd_evt.c
+++ b/osaf/services/saf/immsv/immnd/immnd_evt.c
@@ -1259,7 +1259,7 @@ static uint32_t immnd_evt_proc_oi_att_pu
reqo.attributeNames = evt-info.rtAttUpdRpl.sr.attributeNames;
/*borrowing. */
TRACE_2(oi_att_pull_rpl Before searchInit);
- err = immModel_searchInitialize(cb, reqo, searchOp, SA_FALSE,
SA_FALSE);
+ err = immModel_searchInitialize(cb, reqo, searchOp, SA_FALSE,
SA_TRUE);
if (err == SA_AIS_OK) {
TRACE_2(oi_att_pull_rpl searchInit returned OK,
calling searchNext);
IMMSV_OM_RSP_SEARCH_NEXT *rsp = 0;
--
One dashboard for servers and applications across Physical-Virtual-Cloud
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
Re: [devel] [PATCH 1 of 1] amfd: ignore invalid modification of saAmfSGMaxActiveSIsperSU/saAmfSGMaxStandbySIsperSU [#1361]
Ack, code review only/Thanks HansN
-Original Message-
From: nagendr...@oracle.com [mailto:nagendr...@oracle.com]
Sent: den 21 maj 2015 09:21
To: Hans Nordebäck; praveen.malv...@oracle.com
Cc: opensaf-devel@lists.sourceforge.net
Subject: [PATCH 1 of 1] amfd: ignore invalid modification of
saAmfSGMaxActiveSIsperSU/saAmfSGMaxStandbySIsperSU [#1361]
osaf/services/saf/amf/amfd/sg.cc | 31 +--
1 files changed, 25 insertions(+), 6 deletions(-)
saAmfSGMaxActiveSIsperSU is valid for NPM, N_WAY and N_WAY_ACTIVE and invalid
for Nored and 2N. So, modification of this attribute for Nored and 2N
redudancy models should be ignored.
saAmfSGMaxStandbySIsperSU is valid for NPM and N_WAY and invalid for Nored, 2N
and NwayAct. So, modification of this attribute for Nored, 2N and NwayAct
redudancy models should be ignored.
diff --git a/osaf/services/saf/amf/amfd/sg.cc b/osaf/services/saf/amf/amfd/sg.cc
--- a/osaf/services/saf/amf/amfd/sg.cc
+++ b/osaf/services/saf/amf/amfd/sg.cc
@@ -919,15 +919,34 @@ static void ccb_apply_modify_hdlr(CcbUti
} else if (!strcmp(attribute-attrName,
saAmfSGMaxActiveSIsperSU)) {
if (value_is_deleted)
sg-saAmfSGMaxActiveSIsperSU = -1;
- else
- sg-saAmfSGMaxActiveSIsperSU =
*((SaUint32T *)value);
- TRACE(Modified saAmfSGMaxActiveSIsperSU is
'%u', sg-saAmfSGMaxActiveSIsperSU);
+ else {
+ if ((sg-sg_redundancy_model ==
SA_AMF_NPM_REDUNDANCY_MODEL) ||
+
(sg-sg_redundancy_model == SA_AMF_N_WAY_REDUNDANCY_MODEL) ||
+
(sg-sg_redundancy_model == SA_AMF_N_WAY_ACTIVE_REDUNDANCY_MODEL)) {
+ sg-saAmfSGMaxActiveSIsperSU =
*((SaUint32T *)value);
+ TRACE(Modified
saAmfSGMaxActiveSIsperSU is '%u',
+
sg-saAmfSGMaxActiveSIsperSU);
+ } else {
+ LOG_NO('%s' attribute
saAmfSGMaxActiveSIsperSU not modified,
+not valid for
Nored/2N Redundancy models,
+ sg-name.value);
+ }
+ }
} else if (!strcmp(attribute-attrName,
saAmfSGMaxStandbySIsperSU)) {
if (value_is_deleted)
sg-saAmfSGMaxStandbySIsperSU = -1;
- else
- sg-saAmfSGMaxStandbySIsperSU =
*((SaUint32T *)value);
- TRACE(Modified saAmfSGMaxStandbySIsperSU is
'%u', sg-saAmfSGMaxStandbySIsperSU);
+ else {
+ if ((sg-sg_redundancy_model ==
SA_AMF_NPM_REDUNDANCY_MODEL) ||
+
(sg-sg_redundancy_model == SA_AMF_N_WAY_REDUNDANCY_MODEL)) {
+ sg-saAmfSGMaxStandbySIsperSU =
*((SaUint32T *)value);
+ TRACE(Modified
saAmfSGMaxStandbySIsperSU is '%u',
+
sg-saAmfSGMaxStandbySIsperSU);
+ } else {
+ LOG_NO('%s' attribute
saAmfSGMaxStandbySIsperSU not modified,
+not valid for
Nored/2N/NwayAct Redundancy models,
+ sg-name.value);
+ }
+ }
} else if (!strcmp(attribute-attrName,
saAmfSGAutoAdjustProb)) {
if (value_is_deleted)
sg-saAmfSGAutoAdjustProb =
sg_type-saAmfSgtDefAutoAdjustProb;
--
One dashboard for servers and applications across Physical-Virtual-Cloud
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
