Hi Hans, The current directory structure is in place after thorough review by a wider audience, so let's not deviate from it until it is creating any problem. Code organization is also related to the topic of build and packaging i.e. the sequence in which build proceeds building libraries and binaries, and we don't have to bring-in another level of indirection, so this topic is not entirely about amf or finding a placeholder for gtest. The current structure is also in-line with the commonly used structure of opensource projects.
Also, please note the <topdir>/tests/ is designated for tests. If you wish to differentiate test code, please do so within the <topdir>/tests/ directory. Thanks, Mathi. ----- [email protected] wrote: > Hi Mathi, > > We would like to keep the directory structure as in the patch as the > <topdir>/tests/ are used for function tests, > the unit tests should be placed at the respective service. > > /Thanks HansN > > -----Original Message----- > From: Mathivanan Naickan Palanivelu [mailto:[email protected]] > > Sent: den 5 juni 2015 17:33 > To: Hans Nordebäck > Cc: [email protected]; [email protected]; > [email protected]; Anders Widell; [email protected] > Subject: Re: [PATCH 0 of 2] Review Request for Add unit test V2 > [#1142] > > I dont have other comments other than the below one. > Ack after the above comment is fixed. > > Thanks, > Mathi. > > ----- [email protected] wrote: > > > Hi, > > > > A quick comment. The newly added > > "tests"(osaf/services/saf/amf/amfd/tests/) > > directory is not in accordance to the existing directory structure. > > > > Please move this new directory under the top level tests directory, > > > appropriately. > > say for example <topdir>/tests/amf. > > > > Will get back with additional comments if any by tomorrow. > > > > Thanks, > > Mathi. > > > > ----- [email protected] wrote: > > > > > Summary: Add unit test V2 > > > Review request for Trac Ticket(s): #1142 Peer Reviewer(s): > AndersW, > > > Nagu, Praveen, Mathi Pull request to: > > > Affected branch(es): default > > > Development branch: default > > > > > > -------------------------------- > > > Impacted area Impact y/n > > > -------------------------------- > > > Docs n > > > Build system n > > > RPM/packaging n > > > Configuration files n > > > Startup scripts n > > > SAF services y > > > OpenSAF services n > > > Core libraries n > > > Samples n > > > Tests n > > > Other n > > > > > > > > > Comments (indicate scope for each "y" above): > > > --------------------------------------------- > > > <<EXPLAIN/COMMENT THE PATCH SERIES HERE>> > > > > > > changeset 5503b0cd1d267939d94345bd3d74a23411fe1bf9 > > > Author: Hans Nordeback <[email protected]> > > > Date: Fri, 29 May 2015 10:24:34 +0200 > > > > > > amfd: Add support for google unit test framework V3 [#1142] > > > > > > As part of refactoring enable the use of google unit test > > framework. > > > Updated > > > with review comments > > > > > > changeset 9ec365f1941da5abfcb38208ccb8d4da3d22f9c8 > > > Author: Hans Nordeback <[email protected]> > > > Date: Fri, 29 May 2015 10:32:41 +0200 > > > > > > core: Add unit test for sysf_ipc.c V2 [#1142] > > > > > > Additional unit test program for sysf_ipc as an example on how to > > > > write unit > > > tests on self contained components in openSAF. This is an > exampled > > > that can > > > be extended. Updated with review comments. > > > > > > > > > Added Files: > > > ------------ > > > 00-README.unittest > > > osaf/libs/core/leap/tests/Makefile.am > > > osaf/libs/core/leap/tests/test_sysf_ipc.cc > > > osaf/services/saf/amf/amfd/tests/Makefile.am > > > osaf/services/saf/amf/amfd/tests/test_amf_db.cc > > > > > > > > > Complete diffstat: > > > ------------------ > > > 00-README.unittest | 28 ++ > > > configure.ac | 2 + > > > osaf/libs/core/leap/Makefile.am | 2 +- > > > osaf/libs/core/leap/tests/Makefile.am | 50 +++ > > > osaf/libs/core/leap/tests/test_sysf_ipc.cc | 331 > > > ++++++++++++++++++++++++ > > > osaf/services/saf/amf/amfd/Makefile.am | 2 +- > > > osaf/services/saf/amf/amfd/tests/Makefile.am | 51 +++ > > > osaf/services/saf/amf/amfd/tests/test_amf_db.cc | 61 ++++ > > > 8 files changed, 525 insertions(+), 2 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 Started Linux distro > > > ------------------------------------------- > > > mips n 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 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. ------------------------------------------------------------------------------ _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
