build: Use global variables instead of macros for test features
We don't want the build to depend on recompiling objects for test
separately from other executables. We can tweak settings via global
variables instead.
Change-Id: Ifdd078c2e381848ca1789934a36731eb8ceaf8b1
Signed-off-by: William A. Kennington III <wak@google.com>
diff --git a/config.h.meson b/config.h.meson
index bafd142..7fd16a7 100644
--- a/config.h.meson
+++ b/config.h.meson
@@ -24,13 +24,9 @@
#define SYSTEMD_INTERFACE "org.freedesktop.systemd1.Manager"
#define SYSTEMD_PATH "/org/freedesktop/systemd1"
-#ifdef TESTCASE
-#define ERRLOG_PERSIST_PATH "/tmp/errors"
-#define EXTENSION_PERSIST_DIR "/tmp/extensions"
-#else
-#define ERRLOG_PERSIST_PATH "/var/lib/phosphor-logging/errors"
-#define EXTENSION_PERSIST_DIR "/var/lib/phosphor-logging/extensions"
-#endif
+extern const char *ERRLOG_PERSIST_PATH;
+extern const char *EXTENSION_PERSIST_DIR;
+extern const bool IS_UNIT_TEST;
static constexpr size_t ERROR_CAP = @error_cap@;
static constexpr size_t ERROR_INFO_CAP = @error_info_cap@;
diff --git a/config_main.h b/config_main.h
new file mode 100644
index 0000000..78c223e
--- /dev/null
+++ b/config_main.h
@@ -0,0 +1,5 @@
+#pragma once
+
+const char* ERRLOG_PERSIST_PATH = "/var/lib/phosphor-logging/errors";
+const char* EXTENSION_PERSIST_DIR = "/var/lib/phosphor-logging/extensions";
+const bool IS_UNIT_TEST = false;
diff --git a/extensions/openpower-pels/tools/peltool.cpp b/extensions/openpower-pels/tools/peltool.cpp
index fe3afac..9c034fc 100644
--- a/extensions/openpower-pels/tools/peltool.cpp
+++ b/extensions/openpower-pels/tools/peltool.cpp
@@ -33,6 +33,8 @@
#include <string>
#include <xyz/openbmc_project/Common/File/error.hpp>
+#include "config_main.h"
+
namespace fs = std::filesystem;
using namespace phosphor::logging;
using namespace openpower::pels;
@@ -63,6 +65,11 @@
constexpr auto logging = "/xyz/openbmc_project/logging";
} // namespace object_path
+std::string pelLogDir()
+{
+ return std::string(EXTENSION_PERSIST_DIR) + "/pels/logs";
+}
+
/**
* @brief helper function to get PEL commit timestamp from file name
* @retrun BCDTime - PEL commit timestamp
@@ -288,13 +295,12 @@
std::string val;
char tmpValStr[50];
std::string listStr;
- char name[50];
- sprintf(name, "%.2X%.2X%.2X%.2X%.2X%.2X%.2X%.2X_%.8X", itr.second.yearMSB,
+ char name[51];
+ sprintf(name, "/%.2X%.2X%.2X%.2X%.2X%.2X%.2X%.2X_%.8X", itr.second.yearMSB,
itr.second.yearLSB, itr.second.month, itr.second.day,
itr.second.hour, itr.second.minutes, itr.second.seconds,
itr.second.hundredths, itr.first);
- std::string fileName(name);
- fileName = EXTENSION_PERSIST_DIR "/pels/logs/" + fileName;
+ auto fileName = pelLogDir() + name;
try
{
std::vector<uint8_t> data = getFileData(fileName);
@@ -450,7 +456,7 @@
std::map<uint32_t, BCDTime> PELs;
std::vector<std::string> plugins;
listStr = "{\n";
- for (auto it = fs::directory_iterator(EXTENSION_PERSIST_DIR "/pels/logs");
+ for (auto it = fs::directory_iterator(pelLogDir());
it != fs::directory_iterator(); ++it)
{
if (!fs::is_regular_file((*it).path()))
@@ -539,7 +545,7 @@
bool found = false;
- for (auto it = fs::directory_iterator(EXTENSION_PERSIST_DIR "/pels/logs");
+ for (auto it = fs::directory_iterator(pelLogDir());
it != fs::directory_iterator(); ++it)
{
// The PEL ID is part of the filename, so use that to find the PEL if
@@ -604,7 +610,7 @@
pelID.erase(0, 2);
}
- for (auto it = fs::directory_iterator(EXTENSION_PERSIST_DIR "/pels/logs");
+ for (auto it = fs::directory_iterator(pelLogDir());
it != fs::directory_iterator(); ++it)
{
if (ends_with((*it).path(), pelID))
@@ -621,8 +627,7 @@
{
log<level::INFO>("peltool deleting all event logs");
- for (const auto& entry :
- fs::directory_iterator(EXTENSION_PERSIST_DIR "/pels/logs"))
+ for (const auto& entry : fs::directory_iterator(pelLogDir()))
{
fs::remove(entry.path());
}
@@ -668,7 +673,8 @@
const std::optional<std::regex>& scrubRegex)
{
std::size_t count = 0;
- for (auto it = fs::directory_iterator(EXTENSION_PERSIST_DIR "/pels/logs");
+
+ for (auto it = fs::directory_iterator(pelLogDir());
it != fs::directory_iterator(); ++it)
{
if (!fs::is_regular_file((*it).path()))
diff --git a/log_manager.cpp b/log_manager.cpp
index 83ad168..9e10d75 100644
--- a/log_manager.cpp
+++ b/log_manager.cpp
@@ -89,109 +89,111 @@
// When running as a test-case, the system may have a LOT of journal
// data and we may not have permissions to do some of the journal sync
// operations. Just skip over them.
-#ifndef TESTCASE
-
- constexpr const auto transactionIdVar = "TRANSACTION_ID";
- // Length of 'TRANSACTION_ID' string.
- constexpr const auto transactionIdVarSize = std::strlen(transactionIdVar);
- // Length of 'TRANSACTION_ID=' string.
- constexpr const auto transactionIdVarOffset = transactionIdVarSize + 1;
-
- // Flush all the pending log messages into the journal
- journalSync();
-
- sd_journal* j = nullptr;
- int rc = sd_journal_open(&j, SD_JOURNAL_LOCAL_ONLY);
- if (rc < 0)
+ if (!IS_UNIT_TEST)
{
- logging::log<logging::level::ERR>(
- "Failed to open journal",
- logging::entry("DESCRIPTION=%s", strerror(-rc)));
- return;
- }
+ constexpr const auto transactionIdVar = "TRANSACTION_ID";
+ // Length of 'TRANSACTION_ID' string.
+ constexpr const auto transactionIdVarSize =
+ std::strlen(transactionIdVar);
+ // Length of 'TRANSACTION_ID=' string.
+ constexpr const auto transactionIdVarOffset = transactionIdVarSize + 1;
- std::string transactionIdStr = std::to_string(transactionId);
- std::set<std::string> metalist;
- auto metamap = g_errMetaMap.find(errMsg);
- if (metamap != g_errMetaMap.end())
- {
- metalist.insert(metamap->second.begin(), metamap->second.end());
- }
+ // Flush all the pending log messages into the journal
+ journalSync();
- // Add _PID field information in AdditionalData.
- metalist.insert("_PID");
-
- // Read the journal from the end to get the most recent entry first.
- // The result from the sd_journal_get_data() is of the form VARIABLE=value.
- SD_JOURNAL_FOREACH_BACKWARDS(j)
- {
- const char* data = nullptr;
- size_t length = 0;
-
- // Look for the transaction id metadata variable
- rc = sd_journal_get_data(j, transactionIdVar, (const void**)&data,
- &length);
+ sd_journal* j = nullptr;
+ int rc = sd_journal_open(&j, SD_JOURNAL_LOCAL_ONLY);
if (rc < 0)
{
- // This journal entry does not have the TRANSACTION_ID
- // metadata variable.
- continue;
+ logging::log<logging::level::ERR>(
+ "Failed to open journal",
+ logging::entry("DESCRIPTION=%s", strerror(-rc)));
+ return;
}
- // journald does not guarantee that sd_journal_get_data() returns NULL
- // terminated strings, so need to specify the size to use to compare,
- // use the returned length instead of anything that relies on NULL
- // terminators like strlen().
- // The data variable is in the form of 'TRANSACTION_ID=1234'. Remove
- // the TRANSACTION_ID characters plus the (=) sign to do the comparison.
- // 'data + transactionIdVarOffset' will be in the form of '1234'.
- // 'length - transactionIdVarOffset' will be the length of '1234'.
- if ((length <= (transactionIdVarOffset)) ||
- (transactionIdStr.compare(0, transactionIdStr.size(),
- data + transactionIdVarOffset,
- length - transactionIdVarOffset) != 0))
+ std::string transactionIdStr = std::to_string(transactionId);
+ std::set<std::string> metalist;
+ auto metamap = g_errMetaMap.find(errMsg);
+ if (metamap != g_errMetaMap.end())
{
- // The value of the TRANSACTION_ID metadata is not the requested
- // transaction id number.
- continue;
+ metalist.insert(metamap->second.begin(), metamap->second.end());
}
- // Search for all metadata variables in the current journal entry.
- for (auto i = metalist.cbegin(); i != metalist.cend();)
+ // Add _PID field information in AdditionalData.
+ metalist.insert("_PID");
+
+ // Read the journal from the end to get the most recent entry first.
+ // The result from the sd_journal_get_data() is of the form
+ // VARIABLE=value.
+ SD_JOURNAL_FOREACH_BACKWARDS(j)
{
- rc = sd_journal_get_data(j, (*i).c_str(), (const void**)&data,
+ const char* data = nullptr;
+ size_t length = 0;
+
+ // Look for the transaction id metadata variable
+ rc = sd_journal_get_data(j, transactionIdVar, (const void**)&data,
&length);
if (rc < 0)
{
- // Metadata variable not found, check next metadata variable.
- i++;
+ // This journal entry does not have the TRANSACTION_ID
+ // metadata variable.
continue;
}
- // Metadata variable found, save it and remove it from the set.
- additionalData.emplace_back(data, length);
- i = metalist.erase(i);
- }
- if (metalist.empty())
- {
- // All metadata variables found, break out of journal loop.
- break;
- }
- }
- if (!metalist.empty())
- {
- // Not all the metadata variables were found in the journal.
- for (auto& metaVarStr : metalist)
- {
- logging::log<logging::level::INFO>(
- "Failed to find metadata",
- logging::entry("META_FIELD=%s", metaVarStr.c_str()));
- }
- }
+ // journald does not guarantee that sd_journal_get_data() returns
+ // NULL terminated strings, so need to specify the size to use to
+ // compare, use the returned length instead of anything that relies
+ // on NULL terminators like strlen(). The data variable is in the
+ // form of 'TRANSACTION_ID=1234'. Remove the TRANSACTION_ID
+ // characters plus the (=) sign to do the comparison. 'data +
+ // transactionIdVarOffset' will be in the form of '1234'. 'length -
+ // transactionIdVarOffset' will be the length of '1234'.
+ if ((length <= (transactionIdVarOffset)) ||
+ (transactionIdStr.compare(
+ 0, transactionIdStr.size(), data + transactionIdVarOffset,
+ length - transactionIdVarOffset) != 0))
+ {
+ // The value of the TRANSACTION_ID metadata is not the requested
+ // transaction id number.
+ continue;
+ }
- sd_journal_close(j);
+ // Search for all metadata variables in the current journal entry.
+ for (auto i = metalist.cbegin(); i != metalist.cend();)
+ {
+ rc = sd_journal_get_data(j, (*i).c_str(), (const void**)&data,
+ &length);
+ if (rc < 0)
+ {
+ // Metadata variable not found, check next metadata
+ // variable.
+ i++;
+ continue;
+ }
-#endif
+ // Metadata variable found, save it and remove it from the set.
+ additionalData.emplace_back(data, length);
+ i = metalist.erase(i);
+ }
+ if (metalist.empty())
+ {
+ // All metadata variables found, break out of journal loop.
+ break;
+ }
+ }
+ if (!metalist.empty())
+ {
+ // Not all the metadata variables were found in the journal.
+ for (auto& metaVarStr : metalist)
+ {
+ logging::log<logging::level::INFO>(
+ "Failed to find metadata",
+ logging::entry("META_FIELD=%s", metaVarStr.c_str()));
+ }
+ }
+
+ sd_journal_close(j);
+ }
createEntry(errMsg, errLvl, additionalData);
}
@@ -259,9 +261,10 @@
{
// When running under tests, the Logging.Settings service will not be
// present. Assume false.
-#ifdef TESTCASE
- return false;
-#endif
+ if (IS_UNIT_TEST)
+ {
+ return false;
+ }
std::variant<bool> property;
diff --git a/log_manager_main.cpp b/log_manager_main.cpp
index 6371440..59cbd54 100644
--- a/log_manager_main.cpp
+++ b/log_manager_main.cpp
@@ -8,6 +8,8 @@
#include <sdbusplus/server/manager.hpp>
#include <sdeventplus/event.hpp>
+#include "config_main.h"
+
int main(int /*argc*/, char* /*argv*/[])
{
auto bus = sdbusplus::bus::new_default();
diff --git a/phosphor-rsyslog-config/main.cpp b/phosphor-rsyslog-config/main.cpp
index 3a01e8c..22f7b08 100644
--- a/phosphor-rsyslog-config/main.cpp
+++ b/phosphor-rsyslog-config/main.cpp
@@ -4,6 +4,8 @@
#include <sdbusplus/bus.hpp>
+#include "config_main.h"
+
int main(int /*argc*/, char* /*argv*/[])
{
auto bus = sdbusplus::bus::new_default();
diff --git a/test/common.cpp b/test/common.cpp
new file mode 100644
index 0000000..8905793
--- /dev/null
+++ b/test/common.cpp
@@ -0,0 +1,5 @@
+#include "config.h"
+
+const char* ERRLOG_PERSIST_PATH = "/tmp/errors";
+const char* EXTENSION_PERSIST_DIR = "/tmp/extensions";
+const bool IS_UNIT_TEST = true;
diff --git a/test/meson.build b/test/meson.build
index 1538b7f..6bd81ac 100644
--- a/test/meson.build
+++ b/test/meson.build
@@ -41,13 +41,10 @@
executable(
'test-' + t.underscorify(),
t + '.cpp',
+ 'common.cpp',
log_manager_sources,
'../phosphor-rsyslog-config/server-conf.cpp',
link_with: libphosphor_logging,
- cpp_args: [
- '-DTESTCASE=' + t.underscorify(),
- '-DTESTCASE_' + t.underscorify(),
- ],
dependencies: [
gmock_dep,
gtest_dep,
diff --git a/test/openpower-pels/meson.build b/test/openpower-pels/meson.build
index 3527bed..0c7567e 100644
--- a/test/openpower-pels/meson.build
+++ b/test/openpower-pels/meson.build
@@ -63,10 +63,8 @@
'paths.cpp',
libpel_sources,
peltool_sources,
+ '../common.cpp',
'../../util.cpp',
- cpp_args: [
- '-DTESTCASE=openpower_test_lib',
- ],
include_directories: include_directories(
'../../',
'../../gen',
@@ -89,10 +87,6 @@
libphosphor_logging,
],
link_args: [ '-lpython' + python_ver ],
- cpp_args: [
- '-DTESTCASE=' + t.underscorify(),
- '-DTESTCASE_' + t.underscorify(),
- ],
dependencies: [
gtest_dep,
gmock_dep,