Add unit test stats to output

This change adds some summary data to the output as the unit
tests finish. There is also some "code smell" cleanup identified
by sonar.

Issue-ID: RIC-629

Signed-off-by: E. Scott Daniels <daniels@research.att.com>
Change-Id: Ic0a7629b60981300c2a04f189ffef06cdf6a3c84

diff --git a/CHANGES b/CHANGES
index 9b3e70e..562f096 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -6,6 +6,10 @@
 # squished to one.
 
 release = Cherry
+2020 21 August; version 2.3.2
+       Small cleanup of "code smells" and add additional test stats
+       to unit tests.
+
 2020 21 August; version 2.3.1
        Fix bug in message introduced with "code smell" fix. (RIC-629)
 

diff --git a/CMakeLists.txt b/CMakeLists.txt
index a513b56..eb816d6 100644 (file)
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -30,7 +30,7 @@ cmake_minimum_required( VERSION 3.5 )
 
 set( major_version "2" )               # should be automatically populated from git tag later, but until CI process sets a tag we use this
 set( minor_version "3" )
-set( patch_level "1" )
+set( patch_level "2" )
 
 set( install_root "${CMAKE_INSTALL_PREFIX}" )
 set( install_inc "include/ricxfcpp" )

diff --git a/docs/rel-notes.rst b/docs/rel-notes.rst
index 415d243..d10de52 100644 (file)
--- a/docs/rel-notes.rst
+++ b/docs/rel-notes.rst
@@ -18,6 +18,23 @@ xAPP Framework.
 Cherry Release
 ==============
 
+2020 21 August; version 2.3.2
+-----------------------------
+Small cleanup of "code smells" and add additional test stats
+to unit tests.
+
+
+2020 21 August; version 2.3.1
+-----------------------------
+Fix bug in message introduced with "code smell" fix.
+(RIC-629)
+
+
+2020 20 August; version 2.3.0
+-----------------------------
+Address sonar flagged "bugs" in the config class (RIC-629).
+
+
 2020 13 August; version 2.2.2
 -----------------------------
 Correct potential memory leaks in xapp class (RIC-629)

diff --git a/src/alarm/alarm.cpp b/src/alarm/alarm.cpp
index fc13c7c..c263f7a 100644 (file)
--- a/src/alarm/alarm.cpp
+++ b/src/alarm/alarm.cpp
@@ -32,20 +32,20 @@
 #include <string.h>
 #include <unistd.h>
 #include <time.h>
+#include <iostream>
 
 #include <rmr/RIC_message_types.h>
+
+#include "msg_component.hpp"
+#include "message.hpp"
+#include "alarm.hpp"
+
 #ifndef RIC_ALARM
        // this _should_ come from the message types header, but if not ensure we have something
        constexpr int ric_alarm_value = 110;
        #define RIC_ALARM ric_alarm_value
 #endif
 
-#include <iostream>
-
-#include "msg_component.hpp"
-#include "message.hpp"
-#include "alarm.hpp"
-
 extern const char* __progname;                 // runtime lib supplied since we don't get argv[0]
 
 namespace xapp {
@@ -147,7 +147,6 @@ int xapp::Alarm::build_alarm( int action_id, xapp::Msg_component payload, int pa
                        now()
        );
 
-       //action = std::string( wbuf );
        return used;
 }
 
@@ -246,7 +245,7 @@ xapp::Alarm::Alarm( Alarm&& soi )  :
        ensure the object reference is cleaned up, and ensuring that the source
        object references are removed.
 */
-Alarm& xapp::Alarm::operator=( Alarm&& soi ) {
+Alarm& xapp::Alarm::operator=( Alarm&& soi ) noexcept {
        if( this != &soi ) {                            // cannot do self assignment
                // anything that needs to be freed/delted from soi, must be done here
 
@@ -330,7 +329,7 @@ void xapp::Alarm::Set_whid( int new_whid ) {
        whid = new_whid;
 }
 
-const void xapp::Alarm::Dump() {
+void xapp::Alarm::Dump() const {
        fprintf( stderr, "Alarm: prob id: %d\n", problem_id );
        fprintf( stderr, "Alarm: meid: %s\n", me_id.c_str() );
        fprintf( stderr, "Alarm: app: %s\n", app_id.c_str() );
@@ -343,7 +342,7 @@ const void xapp::Alarm::Dump() {
 /*
        Return the enpoint address string we have.
 */
-const std::string xapp::Alarm::Get_endpoint( ) {
+std::string xapp::Alarm::Get_endpoint( ) const {
        return endpoint;
 }
 
@@ -364,18 +363,18 @@ bool xapp::Alarm::Raise( ) {
        problem ID. Info and addional_info are user supplied data that is just passed
        through.
 */
-bool xapp::Alarm::Raise( int new_severity, int problem, const std::string& info ) {
+bool xapp::Alarm::Raise( int new_severity, int problem, const std::string& cinfo ) {
        Set_severity( new_severity );
        problem_id = problem;
-       this->info = info;
+       info = cinfo;
 
        Raise();
 }
 
-bool xapp::Alarm::Raise( int new_severity, int problem, const std::string& info, const std::string& additional_info ) {
+bool xapp::Alarm::Raise( int new_severity, int problem, const std::string& cinfo, const std::string& additional_info ) {
        Set_severity( new_severity );
        problem_id = problem;
-       this->info = info;
+       info = cinfo;
        this->add_info = additional_info;
 
        Raise();
@@ -397,18 +396,18 @@ bool xapp::Alarm::Clear( ) {
        problem ID. Info and addional_info are user supplied data that is just passed
        through.
 */
-bool xapp::Alarm::Clear( int new_severity, int problem, const std::string& info ) {
+bool xapp::Alarm::Clear( int new_severity, int problem, const std::string& cinfo ) {
        Set_severity( new_severity );
        problem_id = problem;
-       this->info = info;
+       info = cinfo;
 
        Clear();
 }
 
-bool xapp::Alarm::Clear( int new_severity, int problem, const std::string& info, const std::string& additional_info ) {
+bool xapp::Alarm::Clear( int new_severity, int problem, const std::string& cinfo, const std::string& additional_info ) {
        Set_severity( new_severity );
        problem_id = problem;
-       this->info = info;
+       info = cinfo;
        this->add_info = additional_info;
 
        Clear();

diff --git a/src/alarm/alarm.hpp b/src/alarm/alarm.hpp
index 18b9ba2..738dd90 100644 (file)
--- a/src/alarm/alarm.hpp
+++ b/src/alarm/alarm.hpp
@@ -52,7 +52,6 @@ class Alarm {
                std::string app_id = "";                        // application ID
                int                     problem_id  = -1;               // problem ID (specific problem)
                std::string     severity = "";                  // set_sev() xlates from SEV_* consts to collector's string values
-               int                     action = 0;                             // ACT_* constants
                std::string info = "";                          // info string supplied by user
                std::string add_info = "";                      // additional information supplied by user
 
@@ -70,18 +69,18 @@ class Alarm {
                static const int        ACT_CLEAR = 2;
                static const int        ACT_CLEAR_ALL = 3;
 
-               Alarm( std::shared_ptr<Message> msg );          // builders
+               explicit Alarm( std::shared_ptr<Message> msg );         // builders
                Alarm( std::shared_ptr<Message> msg, const std::string& meid  );
                Alarm( std::shared_ptr<Message> msg, int prob_id, const std::string& meid  );
 
                Alarm( const Alarm& soi );                                      // copy to newly created instance
                Alarm& operator=( const Alarm& soi );           // copy operator
                Alarm( Alarm&& soi );                                           // mover
-               Alarm& operator=( Alarm&& soi );                        // move operator
+               Alarm& operator=( Alarm&& soi ) noexcept;                       // move operator
                ~Alarm();                                                                       // destroyer
 
 
-               const std::string Get_endpoint( );
+               std::string Get_endpoint( ) const;
 
                void Set_additional( const std::string& new_info );
                void Set_appid( const std::string& new_id );
@@ -103,7 +102,7 @@ class Alarm {
                bool Clear_all( );
 
 
-               const void Dump();
+               void Dump() const;
 };
 
 } // namespace

diff --git a/src/config/config.cpp b/src/config/config.cpp
index 21b9e65..9d7776d 100644 (file)
--- a/src/config/config.cpp
+++ b/src/config/config.cpp
@@ -91,13 +91,13 @@ void xapp::Config::Listener( ) {
                *tok = 0;
                bname = strdup( tok+1 );
        } else {
-               free( dname );
+               delete dname;
                dname = strdup( "." );
                bname = strdup( fname.c_str() );
        }
 
        wfd = inotify_add_watch( ifd, dname, IN_MOVED_TO | IN_CLOSE_WRITE );            // we only care about close write changes
-       free( dname );
+       delete dname;
 
        if( wfd < 0 ) {
                fprintf( stderr, "<XFCPP> ### ERR ### unable to add watch on config file %s: %s\n", fname.c_str(), strerror( errno ) );
@@ -263,12 +263,10 @@ std::string xapp::Config::Get_control_str( const std::string& name, const std::s
        }
 
        jh->Unset_blob();
-       if( jh->Set_blob( (const char *) "controls" ) ) {
-               if( jh->Exists( name.c_str() ) )  {
-                       value = jh->String( name.c_str() );
-                       if( value.compare( "" ) != 0 ) {
-                               rv = value;
-                       }
+       if(    jh->Set_blob( (const char *) "controls" ) && jh->Exists( name.c_str() ) )  {
+               value = jh->String( name.c_str() );
+               if( value.compare( "" ) != 0 ) {
+                       rv = value;
                }
        }
 

diff --git a/test/metrics_test.cpp b/test/metrics_test.cpp
index 120bbda..1999dc7 100644 (file)
--- a/test/metrics_test.cpp
+++ b/test/metrics_test.cpp
@@ -58,13 +58,17 @@ int main( int argc, char** argv ) {
        set_test_name( "metrics_test" );
 
        x = std::shared_ptr<Xapp>( new Xapp( "4560", true ) );
-       if( x == NULL ) {
-               fprintf( stderr, "<FAIL> unable to allocate xapp object\n" );
+       if( fail_if( x == NULL, "could not allocate new xapp"  ) ) {
                announce_results( 1 );
                return 1;
        }
 
        m = x->Alloc_metrics( );
+       if( fail_if( m == NULL, "could not allocate a metric object" ) ) {
+               announce_results( errors );
+               return errors;
+       }
+
        m->Push_data( "barney_balance", 216.49 );
        m->Push_data( "fred_balance", 760.88 );
        m->Send( );
@@ -80,10 +84,20 @@ int main( int argc, char** argv ) {
 
        // drive alternate builders
        m = x->Alloc_metrics( "different-source" );
+       if( fail_if( m == NULL, "could not allocate a metric object" ) ) {
+               announce_results( errors );
+               return errors;
+       }
+
        m->Push_data( "wilma_balance", 1986.0430 );
        m->Send();
 
        m = x->Alloc_metrics( "different-app", "different-source" );
+       if( fail_if( m == NULL, "could not allocate a metric object" ) ) {
+               announce_results( errors );
+               return errors;
+       }
+
        m->Push_data( "wilma_balance", 1986.0430 );
        m->Push_data( "pebbles_balance", 1982.0614 );
        m->Send();

diff --git a/test/unit_test.sh b/test/unit_test.sh
index b49f81b..fd6f2e2 100755 (executable)
--- a/test/unit_test.sh
+++ b/test/unit_test.sh
@@ -125,6 +125,7 @@ for x in $tests
 do
        ./$x >/tmp/PID$$.log 2>&1
        abort_if_error $? "test failed: $x"
+       grep SUMMARY /tmp/PID$$.log
 done
 
 # it seems that we loose coverage reporting if metrics_test's gcov file is generated

diff --git a/test/ut_support.cpp b/test/ut_support.cpp
index 8e6be90..27fceb5 100644 (file)
--- a/test/ut_support.cpp
+++ b/test/ut_support.cpp
@@ -28,6 +28,9 @@
 #include <string>
 
 static std::string test_name = "unknown";
+static int ut_tests_driven = 0;
+
+// -------------------------------------------------------------------------------
 
 /*
        Set the name of the current tester
@@ -40,6 +43,8 @@ extern void set_test_name( std::string name ) {
        Returns 1 if the condition is true (not zero)
 */
 extern int fail_if( int cond, std::string reason ) {
+       ut_tests_driven++;
+
        if( cond ) {
                fprintf( stderr, "<FAIL> %s: %s\n", test_name.c_str(), reason.c_str() );
                return 1;
@@ -52,6 +57,8 @@ extern int fail_if( int cond, std::string reason ) {
        Returns 1 if the condition is false.
 */
 extern int fail_if_false( int cond, std::string reason ) {
+       ut_tests_driven++;
+
        if( !cond ) {
                fprintf( stderr, "<FAIL> %s: %s\n", test_name.c_str(), reason.c_str() );
                return 1;
@@ -61,6 +68,9 @@ extern int fail_if_false( int cond, std::string reason ) {
 }
 
 extern void announce_results( int errors ) {
+       fprintf( stderr, "<SUMMARY> %s %d tests drivn, %d passed, %d failed\n",
+               test_name.c_str(), ut_tests_driven, ut_tests_driven - errors, errors );
+
        if( errors > 0 ) {
                fprintf( stderr, "<FAIL> %s: failed with %d errors\n", test_name.c_str(), errors );
        } else {