From f5b1bb06f2027328134c71a90e27c40c626fb19b Mon Sep 17 00:00:00 2001 From: AlesKus <122485829+AlesKus@users.noreply.github.com> Date: Tue, 7 Jan 2025 04:38:47 +0300 Subject: [PATCH] Remove local_time from ActiveTextLogger. Fixes: #2815 (#3104) * ActiveTextLogger: make tests runnable * ActiveTextLogger: fix unit tests * ActiveTextLogger: use camelCase for test names * ActiveTextLogger: add test for workstation timestamp * ActiveTextLogger: remove logging of local workstation time (issue #2815) * ActiveTextLogger: mention file truncation in the SDD * Changing PRId32 to PRIu32 for time fields. * Assert of string compare to ASSERT_STREQ --------- Co-authored-by: M Starch --- Svc/ActiveTextLogger/ActiveTextLogger.cpp | 30 ++--------- Svc/ActiveTextLogger/CMakeLists.txt | 2 +- Svc/ActiveTextLogger/LogFile.cpp | 2 +- Svc/ActiveTextLogger/docs/sdd.md | 4 +- .../test/ut/ActiveTextLoggerTestMain.cpp | 28 +++++++++++ .../test/ut/ActiveTextLoggerTester.cpp | 50 ++++++++++++++++--- .../test/ut/ActiveTextLoggerTester.hpp | 5 +- 7 files changed, 83 insertions(+), 38 deletions(-) create mode 100644 Svc/ActiveTextLogger/test/ut/ActiveTextLoggerTestMain.cpp diff --git a/Svc/ActiveTextLogger/ActiveTextLogger.cpp b/Svc/ActiveTextLogger/ActiveTextLogger.cpp index 0b5a5e8cdf..4eb5f1c571 100644 --- a/Svc/ActiveTextLogger/ActiveTextLogger.cpp +++ b/Svc/ActiveTextLogger/ActiveTextLogger.cpp @@ -76,31 +76,11 @@ namespace Svc { // TODO: Add calling task id to format string char textStr[FW_INTERNAL_INTERFACE_STRING_MAX_SIZE]; - if (timeTag.getTimeBase() == TB_WORKSTATION_TIME) { - - time_t t = timeTag.getSeconds(); - // Using localtime_r prevents any other calls to localtime (from another thread for example) from - // interfering with our time object before we use it. However, the null pointer check is still needed - // to ensure a successful call - tm tm; - if (localtime_r(&t, &tm) == nullptr) { - return; - } - - (void) snprintf(textStr, - FW_INTERNAL_INTERFACE_STRING_MAX_SIZE, - "EVENT: (%" PRI_FwEventIdType ") (%04d-%02d-%02dT%02d:%02d:%02d.%06" PRIu32 ") %s: %s\n", - id, tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday, tm.tm_hour, - tm.tm_min,tm.tm_sec,timeTag.getUSeconds(), - severityString,text.toChar()); - } - else { - - (void) snprintf(textStr, - FW_INTERNAL_INTERFACE_STRING_MAX_SIZE, - "EVENT: (%" PRI_FwEventIdType ") (%" PRI_FwTimeBaseStoreType ":%" PRId32 ",%" PRId32 ") %s: %s\n", - id, static_cast(timeTag.getTimeBase()),timeTag.getSeconds(),timeTag.getUSeconds(),severityString,text.toChar()); - } + (void) snprintf(textStr, + FW_INTERNAL_INTERFACE_STRING_MAX_SIZE, + "EVENT: (%" PRI_FwEventIdType ") (%" PRI_FwTimeBaseStoreType ":%" PRIu32 ",%" PRIu32 ") %s: %s\n", + id, static_cast(timeTag.getTimeBase()), timeTag.getSeconds(), timeTag.getUSeconds(), + severityString, text.toChar()); // Call internal interface so that everything else is done on component thread, // this helps ensure consistent ordering of the printed text: diff --git a/Svc/ActiveTextLogger/CMakeLists.txt b/Svc/ActiveTextLogger/CMakeLists.txt index c0f372cf8d..f5a6d3d3c4 100644 --- a/Svc/ActiveTextLogger/CMakeLists.txt +++ b/Svc/ActiveTextLogger/CMakeLists.txt @@ -17,6 +17,6 @@ register_fprime_module() set(UT_SOURCE_FILES "${FPRIME_FRAMEWORK_PATH}/Svc/ActiveTextLogger/ActiveTextLogger.fpp" "${CMAKE_CURRENT_LIST_DIR}/test/ut/ActiveTextLoggerTester.cpp" - "${CMAKE_CURRENT_LIST_DIR}/test/ut/ActiveTextLoggerTester.cpp" + "${CMAKE_CURRENT_LIST_DIR}/test/ut/ActiveTextLoggerTestMain.cpp" ) register_fprime_ut() diff --git a/Svc/ActiveTextLogger/LogFile.cpp b/Svc/ActiveTextLogger/LogFile.cpp index 927762f46f..0d7154cca7 100644 --- a/Svc/ActiveTextLogger/LogFile.cpp +++ b/Svc/ActiveTextLogger/LogFile.cpp @@ -140,7 +140,7 @@ namespace Svc { } // Open the file (using CREATE so that it truncates an already existing file): - Os::File::Status stat = this->m_file.open(fileNameFinal, Os::File::OPEN_CREATE, Os::File::OverwriteType::NO_OVERWRITE); + Os::File::Status stat = this->m_file.open(fileNameFinal, Os::File::OPEN_CREATE, Os::File::OverwriteType::OVERWRITE); // Bad status when trying to open the file: if (stat != Os::File::OP_OK) { diff --git a/Svc/ActiveTextLogger/docs/sdd.md b/Svc/ActiveTextLogger/docs/sdd.md index 80ab267bbd..5ccd114431 100644 --- a/Svc/ActiveTextLogger/docs/sdd.md +++ b/Svc/ActiveTextLogger/docs/sdd.md @@ -16,7 +16,7 @@ ISF-ATL-002 | The `Svc::ActiveTextLogger` component shall write received log tex ISF-ATL-003 | The `Svc::ActiveTextLogger` component shall format the passed log text on the calling thread context, and perform all other processing on the component's thread. | Inspection ISF-ATL-004 | The `Svc::ActiveTextLogger` component shall stop writing to the optional file if it would exceed its max size. | Unit Test ISF-ATL-005 | The `Svc::ActiveTextLogger` component shall provide a public method to supply the filename to write to and max size. | Unit Test -ISF-ATL-006 | The `Svc::ActiveTextLogger` component shall attempt to create a new file to write to if the supplied one already exists. It will try up to ten times, by adding an integer suffix to the filename, ie "file","file0","file1"..."file9" | Unit Test +ISF-ATL-006 | The `Svc::ActiveTextLogger` component shall attempt to create a new file to write to if the supplied one already exists. It will try up to ten times, by adding an integer suffix to the filename, ie "file","file0","file1"..."file9". After the last possible attempt is failed, the initial file shall be overwritten; ie if "file9" already exists, the "file" shall be truncated and used as a log file. | Unit Test ## 3. Design @@ -45,7 +45,7 @@ The `Svc::ActiveTextLogger` component provides a text logging function for the s Once a valid file and max size is supplied via a public function call, the `Svc::ActiveTextLogger` component writes to that file as well as standard output. The `Svc::ActiveTextLogger` component will stop writing to the file if it would exceed the maximum size. -If the file supplied already exists, the `Svc::ActiveTextLogger` component will attempt to create a new file up to ten times by appending an integer suffix to end of the file name. +If the file supplied already exists, the `Svc::ActiveTextLogger` component will attempt to create a new file up to ten times by appending an integer suffix to end of the file name. After the last possible attempt is failed, the initial file is overwritten. ### 3.3 Scenarios diff --git a/Svc/ActiveTextLogger/test/ut/ActiveTextLoggerTestMain.cpp b/Svc/ActiveTextLogger/test/ut/ActiveTextLoggerTestMain.cpp new file mode 100644 index 0000000000..529270f3b8 --- /dev/null +++ b/Svc/ActiveTextLogger/test/ut/ActiveTextLoggerTestMain.cpp @@ -0,0 +1,28 @@ +// ====================================================================== +// \title ActiveTextLoggerTestMain.cpp +// \author AlesKus +// \brief cpp file for ActiveTextLogger component test main function +// ====================================================================== + +#include "ActiveTextLoggerTester.hpp" + + +TEST(Nominal, Logging) { + Svc::ActiveTextLoggerTester tester; + tester.runNominalTest(); +} + +TEST(Nominal, WorkstationTime) { + Svc::ActiveTextLoggerTester tester; + tester.testWorkstationTimestamp(); +} + +TEST(OffNominal, FileHandling) { + Svc::ActiveTextLoggerTester tester; + tester.runOffNominalTest(); +} + +int main(int argc, char** argv) { + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} diff --git a/Svc/ActiveTextLogger/test/ut/ActiveTextLoggerTester.cpp b/Svc/ActiveTextLogger/test/ut/ActiveTextLoggerTester.cpp index 01314bac68..25c951e1ff 100644 --- a/Svc/ActiveTextLogger/test/ut/ActiveTextLoggerTester.cpp +++ b/Svc/ActiveTextLogger/test/ut/ActiveTextLoggerTester.cpp @@ -38,7 +38,7 @@ namespace Svc { // ---------------------------------------------------------------------- void ActiveTextLoggerTester :: - run_nominal_test() + runNominalTest() { printf("Testing writing to console\n"); @@ -106,7 +106,7 @@ namespace Svc { snprintf(textStr, sizeof(textStr), "EVENT: (%d) (%d:%d,%d) %s: %s", id,timeTag.getTimeBase(),timeTag.getSeconds(),timeTag.getUSeconds(),severityString,text.toChar()); - ASSERT_EQ(0,strcmp(textStr,buf)); + ASSERT_STREQ(textStr,buf); (void) Fw::StringUtils::string_copy(oldLine, buf, static_cast(sizeof(oldLine))); } } @@ -160,7 +160,7 @@ namespace Svc { } void ActiveTextLoggerTester :: - run_off_nominal_test() + runOffNominalTest() { // TODO file errors- use the Os/Stubs? @@ -327,7 +327,6 @@ namespace Svc { // Create 11th which will fail and re-use the original: stat = this->component.set_log_file(baseName,50); - snprintf(baseNameWithSuffix, sizeof(baseNameWithSuffix), "%s%d",baseName,i); ASSERT_TRUE(stat); ASSERT_TRUE(this->component.m_log_file.m_openFile); printf("<< %s %s\n",baseName,this->component.m_log_file.m_fileName.toChar()); @@ -347,6 +346,46 @@ namespace Svc { } + void ActiveTextLoggerTester :: + testWorkstationTimestamp() + { + printf("Testing workstation timestamp\n"); + + // Setup file for writing to: + const char* logFileName = "test_file"; + bool stat = this->component.set_log_file(logFileName,512); + ASSERT_TRUE(stat); + ASSERT_TRUE(this->component.m_log_file.m_openFile); + + // Log message: + FwEventIdType id = 1; + Fw::Time timeTag(TB_WORKSTATION_TIME, 3, 6); + Fw::LogSeverity severity = Fw::LogSeverity::ACTIVITY_HI; + const char* severityString = "ACTIVITY_HI"; + Fw::TextLogString text("This line has a valid timestamp."); + this->invoke_to_TextLogger(0, id, timeTag, severity, text); + this->component.doDispatch(); + + // Read file to verify contents: + std::ifstream logStream(logFileName); + while(logStream) { + char buf[256]; + logStream.getline(buf, 256); + if (logStream) { + std::cout << "readLine: " << buf << std::endl; + char textStr[512]; + snprintf(textStr, sizeof(textStr), + "EVENT: (%d) (%d:%d,%d) %s: %s", + id,timeTag.getTimeBase(), timeTag.getSeconds(), timeTag.getUSeconds(), severityString, text.toChar()); + ASSERT_EQ(0, strcmp(textStr, buf)); + } + } + logStream.close(); + + // Clean up: + remove(logFileName); + } + // ---------------------------------------------------------------------- // Helper methods // ---------------------------------------------------------------------- @@ -361,9 +400,6 @@ namespace Svc { this->component.get_TextLogger_InputPort(0) ); - - - } void ActiveTextLoggerTester :: diff --git a/Svc/ActiveTextLogger/test/ut/ActiveTextLoggerTester.hpp b/Svc/ActiveTextLogger/test/ut/ActiveTextLoggerTester.hpp index 1ef3e77a03..9bafffdcc1 100644 --- a/Svc/ActiveTextLogger/test/ut/ActiveTextLoggerTester.hpp +++ b/Svc/ActiveTextLogger/test/ut/ActiveTextLoggerTester.hpp @@ -35,8 +35,9 @@ namespace Svc { // Tests // ---------------------------------------------------------------------- - void run_nominal_test(); - void run_off_nominal_test(); + void runNominalTest(); + void runOffNominalTest(); + void testWorkstationTimestamp(); private: