Feature/usblogupload backup by Abhinavpv28 · Pull Request #87 · rdkcentral/dcm-agent

Abhinavpv28 · 2026-03-02T03:36:57Z

No description provided.

Added unit tests for usb_log_utils functions including initialization, timestamp retrieval, filesystem sync, and file copy operations.

Remove unnecessary echo command from script

Add RDK_PROFILE to device properties and update includes

Removed usb_log_main_gtest from test execution.

Remove coverage report listing from unit_test.sh

rdkcmf-jenkins · 2026-03-02T03:38:39Z

b'## Copyright scan failure
Commit: f159ad9
Report detail: https://gist.github.com/rdkcmf-jenkins/543dfb4ac77caf5939e26ed8e35bda47'

rdkcmf-jenkins · 2026-03-02T03:39:09Z

b'## Blackduck scan failure details

Summary: 0 violations, 0 files pending approval, 2 files pending identification.

Protex Server Path: /home/blackduck/github/dcm-agent/87/rdkcentral/dcm-agent
Commit: f159ad9

Report detail: gist'

Copilot

Pull request overview

This PR expands the usbLogUpload unit test suite and its build/run plumbing, adding new Google Test binaries for utility and archive modules and integrating them into the repository’s unit test runner.

Changes:

Added new GTest executables for usb_log_utils and usb_log_archive, and updated existing usbLogUpload tests.
Updated usbLogUpload/unittest autotools files (configure.ac, Makefile.am) to build/link the new test binaries.
Updated unit_test.sh to build and execute the new usbLogUpload unit tests and to enable coverage by default.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 14 comments.

Show a summary per file

File	Description
usbLogUpload/unittest/usb_log_validation_gtest.cpp	Refactors validation tests; replaces a device-compatibility test with a placeholder.
usbLogUpload/unittest/usb_log_utils_gtest.cpp	Adds new unit tests for `usb_log_utils.c` with stubs for external dependencies.
usbLogUpload/unittest/usb_log_main_gtest.cpp	Adds mocks include and custom `main()` plus cleanup for global mocks.
usbLogUpload/unittest/usb_log_file_manager_gtest.cpp	Adjusts file-manager tests, adds a local `remove_directory()` stub, and adds custom `main()`.
usbLogUpload/unittest/usb_log_archive_gtest.cpp	Adds new unit tests for `usb_log_archive.c` with dependency stubs.
usbLogUpload/unittest/configure.ac	Adds a new autotools `configure.ac` for the `usbLogUpload` unit test subproject.
usbLogUpload/unittest/Makefile.am	Adds new test binaries and updates link inputs/libs.
usbLogUpload/src/usb_log_main.c	Wraps the production `main()` in `#ifndef GTEST_ENABLE` to allow test builds.
usbLogUpload/include/usb_log_validation.h	Removes `validate_system_prerequisites()` declaration from the public header.
unit_test.sh	Enables coverage by default; builds and runs the new `usbLogUpload` test binaries; sets runtime env.

Comments suppressed due to low confidence (1)

unit_test.sh:25

ENABLE_COV is now set to true by default, which makes the --enable-cov flag effectively redundant and forces coverage tooling on every run. If coverage is meant to be optional, restore the default to false and only enable it when --enable-cov is provided.

ENABLE_COV=true

if [ "x$1" = "x--enable-cov" ]; then
      echo "Enabling coverage options"
      export CXXFLAGS="-g -O0 -fprofile-arcs -ftest-coverage"

Copilot · 2026-03-02T03:48:31Z

 export top_srcdir=`pwd`
-
+export LD_LIBRARY_PATH="/usr/local/lib:$TOP_DIR/uploadstblogs/src/.libs:$LD_LIBRARY_PATH"
+echo "RDK_PROFILE=TV" >> /etc/device.properties


unit_test.sh appends RDK_PROFILE=TV to /etc/device.properties unconditionally. This can create duplicate entries on repeated runs and makes test setup non-idempotent. Follow the pattern used elsewhere in the repo (grep before append, and create the file if missing) to keep the environment stable.

Suggested change

echo "RDK_PROFILE=TV" >> /etc/device.properties

if [ ! -f /etc/device.properties ]; then

touch /etc/device.properties

fi

if ! grep -q '^RDK_PROFILE=TV$' /etc/device.properties 2>/dev/null; then

echo "RDK_PROFILE=TV" >> /etc/device.properties

fi

Copilot · 2026-03-02T03:48:32Z

+
+// Mocks and stubs for dependencies
+extern "C" {
+    int get_current_timestamp(char *buf, size_t len) {
+        strncpy(buf, "01/01/26-12:00:00", len-1);
+        buf[len-1] = '\0';


This file uses strncpy in the stubbed get_current_timestamp() but does not include <cstring>/<string.h>. Some toolchains will fail to compile due to a missing declaration. Add the appropriate header (and consider using std::strncpy when including <cstring>).

Suggested change

// Mocks and stubs for dependencies

extern "C" {

int get_current_timestamp(char *buf, size_t len) {

strncpy(buf, "01/01/26-12:00:00", len-1);

buf[len-1] = '\0';

#include <cstring>

// Mocks and stubs for dependencies

extern "C" {

int get_current_timestamp(char *buf, size_t len) {

std::strncpy(buf, "01/01/26-12:00:00", len - 1);

buf[len - 1] = '\0';

Copilot · 2026-03-02T03:48:32Z

+cd ../..
+sh cov_build.sh
+cd -


cov_build.sh is invoked unconditionally. This script clones/builds dependencies and adds network + time cost to every unit test run, even when coverage isn’t requested. Consider gating this behind the coverage flag (or a separate explicit option) and/or caching dependencies in CI.

Copilot · 2026-03-02T03:48:32Z

 /**
 * @brief Test temporary directory creation with NULL buffer
 */


There is no longer a success-path test for create_temporary_directory() (the previous success test was removed), leaving only error-path coverage below. Add back a positive test that verifies the function returns 0, produces a valid path, and that the directory is created and cleaned up.

Copilot · 2026-03-02T03:48:33Z

 TEST_F(UsbLogValidationTest, DeviceCompatibilityInvalidTest) {
-    // TODO: Test validate_device_compatibility with non-PLATCO device
-    // This would require mocking environment variables or config
-    EXPECT_TRUE(true); // Placeholder
-}
-
-/**
- * @brief Test USB mount point validation with valid path
- */
-TEST_F(UsbLogValidationTest, UsbMountPointValidTest) {
-    // TODO: Test validate_usb_mount_point with valid path
-    const char* valid_path = "/tmp";
-    EXPECT_EQ(validate_usb_mount_point(valid_path), 0);
+    EXPECT_TRUE(true); 
 }


DeviceCompatibilityInvalidTest is currently a placeholder (EXPECT_TRUE(true)) and no longer exercises validate_device_compatibility(). This reduces coverage of a key gate in usb_log_main.c. Consider stubbing/mocking getDevicePropertyData("RDK_PROFILE", ...) to return a non-"TV" value and asserting the expected non-zero return code.

Copilot · 2026-03-02T03:48:34Z

    char* test_argv[] = {(char*)"usblogupload", (char*)"/tmp/test_usb"};
    // This would require refactoring main to be testable
    EXPECT_TRUE(true); // Placeholder
-}
+}


MainArgumentValidationTest is still a placeholder, and the file also contains tests that call usb_log_upload_execute() without preparing the required filesystem state (mount point directory must exist, log paths must be set up, etc.). As written, the suite is likely to be flaky/fail on a clean CI runner. Either create/cleanup the required directories in the fixture or mock the filesystem + validation dependencies so assertions are deterministic.

Copilot · 2026-03-02T03:48:34Z

+bool remove_directory(const char* dirpath) {
+    if (!dirpath) return false;
+    struct stat st;
+    // Return false if directory does not exist
+    if (stat(dirpath, &st) != 0 || !S_ISDIR(st.st_mode)) return false;


The test-local remove_directory() stub returns false when the directory does not exist, which differs from the production remove_directory() used by cleanup_temporary_files() (it treats “already removed” as success). This makes the unit tests validate behavior that the shipped code does not have. Align the stub semantics with production or explicitly mock remove_directory via the shared mock layer.

Copilot · 2026-03-02T03:48:34Z

    // Directory should not exist yet
    EXPECT_FALSE(access(usb_log_dir.c_str(), F_OK) == 0);
-    
+
    // Create directory should succeed
    EXPECT_EQ(create_usb_log_directory(usb_log_dir.c_str()), 0);


CreateUsbLogDirectorySuccessTest no longer verifies that the directory was actually created, and CleanupTemporaryFilesSuccessTest no longer verifies the directory was removed. With the current stubs, these tests can pass even if nothing is created/cleaned up. Add assertions on filesystem state (or use mocks that simulate state changes) so the tests validate observable behavior.

Copilot · 2026-03-02T03:48:35Z

+TEST(UsbLogUtilsTest, CopyFileAndDelete_Success) {
+    const char* src = "test_src.txt";
+    const char* dst = "test_dst.txt";
+    FILE* f = fopen(src, "w");


CopyFileAndDelete_Success calls fputs() on f without checking whether fopen() succeeded. If the working directory is not writable, this will crash the test instead of failing cleanly. Add an ASSERT_NE(f, nullptr) (or equivalent) before writing.

Suggested change

FILE* f = fopen(src, "w");

FILE* f = fopen(src, "w");

ASSERT_NE(f, nullptr);

Copilot · 2026-03-02T03:48:35Z


 cd ../uploadstblogs/unittest
+cd ../..
+sh cov_build.sh


The new call to sh cov_build.sh introduces a supply-chain risk because cov_build.sh clones and executes remote GitHub repositories (e.g., rdkcentral/iarmmgrs, rdkcentral/rdk_logger, rdkcentral/telemetry, rdkcentral/common_utilities) using mutable branches without any integrity or version pinning. If any of those upstream repositories or the network path is compromised, an attacker could execute arbitrary code in your CI/test environment and potentially access secrets or tamper with build artifacts. To mitigate this, pin git clone operations to specific commit hashes or verified release tags and/or vendor the required build artifacts instead of executing unpinned remote code during the test run.

mhughesacn · 2026-03-02T12:34:07Z

Hi @Abhinavpv28 : New code files should have standard Comcast headers please.

Abhinavpv28 added 30 commits February 20, 2026 17:15

Update usb_log_validation.h

93fa87b

Update usb_log_validation_gtest.cpp

3a561b2

Add conditional compilation for GTEST_ENABLE

2348d03

Update Makefile.am

8e0faaa

Update Makefile.am

7ca8df1

Update usb_log_validation_gtest.cpp

fafcdb8

Update usb_log_file_manager_gtest.cpp

210f256

Update usb_log_main_gtest.cpp

806dd7e

Update unit_test.sh

df811f3

Update unit_test.sh

17de9ff

Update unit_test.sh

96c4b9d

Update unit_test.sh

56670aa

Update unit_test.sh

75604ac

Update unit_test.sh

c1585ab

Update directory path for unit tests

cc12250

Update unit_test.sh

7b533c3

Update unit_test.sh

20d0301

Update unit_test.sh

e72dba1

Create configure.ac

78678fa

Update Makefile.am

1504714

Update Makefile.am

dfdc0bd

Update Makefile.am

d78399d

Update Makefile.am

18a6948

Update usb_log_file_manager_gtest.cpp

254e48c

Update usb_log_main_gtest.cpp

131b615

Update usb_log_main_gtest.cpp

1089a56

Update usb_log_file_manager_gtest.cpp

519cf24

Update usb_log_main_gtest.cpp

a21a5c7

Update usb_log_main_gtest.cpp

6821e5f

Update unit_test.sh

0ca2a37

Abhinavpv28 added 23 commits February 25, 2026 21:13

Update usb_log_file_manager_gtest.cpp

3f29a1f

Update usb_log_file_manager_gtest.cpp

e1ea0bf

Update unit_test.sh

6bcffab

Create usb_log_archive_gtest.cpp

62aad49

Update Makefile.am

80b482c

Update Makefile.am

03e3c26

Implement unit tests for usb_log_utils

b818fe9

Added unit tests for usb_log_utils functions including initialization, timestamp retrieval, filesystem sync, and file copy operations.

Update Makefile.am

d79e1f5

Update unit_test.sh

d96f7ab

Remove unnecessary empty line in unit_test.sh

8418acb

Update unit_test.sh

57b9f76

Update unit_test.sh

995120d

Remove echo command from unit_test.sh

aad24c0

Remove unnecessary echo command from script

Update unit_test.sh to set RDK_PROFILE

8c25027

Add RDK_PROFILE to device properties and update includes

Update usb_log_file_manager_gtest.cpp

e826ce4

Update usb_log_file_manager_gtest.cpp

553c5ba

Remove usb_log_main_gtest from unit tests

51888c5

Removed usb_log_main_gtest from test execution.

Update usb_log_file_manager_gtest.cpp

01546fd

Update usb_log_validation_gtest.cpp

8cea413

Update usb_log_validation_gtest.cpp

d39cee0

Update usb_log_validation_gtest.cpp

6bf0533

Enable coverage options in unit_test.sh

4940298

Remove lcov list command from coverage report

f159ad9

Remove coverage report listing from unit_test.sh

Copilot AI review requested due to automatic review settings March 2, 2026 03:36

Abhinavpv28 requested a review from a team as a code owner March 2, 2026 03:36

Copilot started reviewing on behalf of Abhinavpv28 March 2, 2026 03:37 View session

Copilot AI reviewed Mar 2, 2026

View reviewed changes

-echo "RDK_PROFILE=TV" >> /etc/device.properties
+if [ ! -f /etc/device.properties ]; then
+    touch /etc/device.properties
+fi
+if ! grep -q '^RDK_PROFILE=TV$' /etc/device.properties 2>/dev/null; then
+    echo "RDK_PROFILE=TV" >> /etc/device.properties
+fi

	FILE* f = fopen(src, "w");
	FILE* f = fopen(src, "w");
	ASSERT_NE(f, nullptr);

Uh oh!

Conversation

Abhinavpv28 commented Mar 2, 2026

Uh oh!

rdkcmf-jenkins commented Mar 2, 2026

Uh oh!

rdkcmf-jenkins commented Mar 2, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

mhughesacn commented Mar 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants