RDK-60519: Adding L1 unit test cases for scheduler #238

rosemarybennyy · 2026-01-20T12:02:27Z

Reason for change: Adding L1 unit test cases for scheduler
Test Procedure: Tested and verified
Risks: Medium
Priority: P1

Copilot

Pull request overview

This PR adds L1 (Level 1) unit test cases for the scheduler component to increase code coverage. The changes primarily add new test cases covering edge cases, error handling paths, and various function behaviors that were previously untested.

Changes:

Added 20+ new test cases covering getLapsedTime, registerProfileWithScheduler, unregisterProfileFromScheduler, SendInterruptToTimeoutThread, uninitScheduler, TimeoutThread, and freeSchedulerProfile functions
Modified an existing test (TIMEOUTTHREAD/TEST1) to use non-zero timeRefinSec value
Added a callback function wrapper in scheduler.c for indirect function pointer testing under GTEST_ENABLE flag

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 16 comments.

File	Description
source/test/scheduler/schedulerTest.cpp	Adds comprehensive L1 unit test cases for scheduler functions, including edge cases, error conditions, and branch coverage tests
source/scheduler/scheduler.c	Adds test-only callback function wrapper for getSchdInSec to enable indirect function pointer testing

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

source/test/scheduler/schedulerTest.cpp

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 12 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

source/test/scheduler/schedulerTest.cpp

Reason for change: Adding L1 unit test cases for scheduler Test Procedure: Tested and verified Risks: Medium Priority: P1 Signed-off-by: Rose Mary Benny <RoseMary_Benny@comcast.com>

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-20T22:07:49Z

source/test/scheduler/schedulerTest.cpp

+  typedef unsigned int (*getSchdInSec_fn)(char*);
+  getSchdInSec_fn getSchdInSec_fnCallback(void);
+}
+TEST(getSchdInSec, IndirectCall)
+{
+    getSchdInSec_fn fn = getSchdInSec_fnCallback();


Duplicate typedef definition. The typedef for getSchdInSec_fn is defined identically in both the test file (line 439) and the source file (line 683). This duplication violates the DRY principle and could lead to maintenance issues if the signature needs to change. Consider defining this typedef in a shared header file that both files can include when GTEST_ENABLE is defined.

Suggested change

typedef unsigned int (*getSchdInSec_fn)(char*);

getSchdInSec_fn getSchdInSec_fnCallback(void);

}

TEST(getSchdInSec, IndirectCall)

{

getSchdInSec_fn fn = getSchdInSec_fnCallback();

unsigned int (*getSchdInSec_fnCallback(void))(char*);

}

TEST(getSchdInSec, IndirectCall)

{

auto fn = getSchdInSec_fnCallback();

Copilot · 2026-01-20T22:07:50Z

source/test/scheduler/schedulerTest.cpp

+{
+    initScheduler((TimeoutNotificationCB)ReportProfiles_ToutCb, (ActivationTimeoutCB)ReportProfiles_ActivationToutCb, (NotifySchedulerstartCB)NotifySchedulerstartCb);
+
+


Excessive blank lines. There are two consecutive blank lines (306-307) which is inconsistent with the rest of the codebase where single blank lines are used for separation. Remove one of these blank lines for consistency.

Suggested change

Copilot · 2026-01-20T22:07:50Z

source/test/scheduler/schedulerTest.cpp

+    time1.tv_nsec = 200000000; // 2.1 seconds in nanoseconds
+    time2.tv_sec = 10;
+    time2.tv_nsec = 100000000;     // 0.001 seconds in nanoseconds


The comment at line 170 is inaccurate. The value 100000000 nanoseconds equals 0.1 seconds, not 0.001 seconds.

Suggested change

time1.tv_nsec = 200000000; // 2.1 seconds in nanoseconds

time2.tv_sec = 10;

time2.tv_nsec = 100000000; // 0.001 seconds in nanoseconds

time1.tv_nsec = 200000000; // 0.2 seconds in nanoseconds

time2.tv_sec = 10;

time2.tv_nsec = 100000000; // 0.1 seconds in nanoseconds

Copilot · 2026-01-20T22:07:50Z

source/test/scheduler/schedulerTest.cpp

+    pthread_mutex_init(&sch->tMutex, NULL);
+    pthread_cond_init(&sch->tCond, NULL);
+    pthread_t threadId;
+    ASSERT_EQ(0, pthread_create(&threadId, NULL, DummySchedulerThread, NULL));	


Inconsistent whitespace in ASSERT_EQ call. The line has trailing whitespace after the semicolon that should be removed for consistency with the rest of the codebase.

Suggested change

ASSERT_EQ(0, pthread_create(&threadId, NULL, DummySchedulerThread, NULL));

ASSERT_EQ(0, pthread_create(&threadId, NULL, DummySchedulerThread, NULL));

Copilot · 2026-01-20T22:07:51Z

source/test/scheduler/schedulerTest.cpp

+    pthread_mutex_init(&sch->tMutex, NULL);
+    pthread_cond_init(&sch->tCond, NULL);
+    pthread_t threadId;
+    ASSERT_EQ(0, pthread_create(&threadId, NULL, DummySchedulerThread, NULL));	


Potential resource leak. The test creates a thread with pthread_create but never joins or properly cleans it up. The thread is created but not associated with the SchedulerProfile structure (sch->tId is not set), and the DummySchedulerThread returns immediately. While freeSchedulerProfile calls pthread_detach, it detaches sch->tId which was never initialized, not the threadId variable that was actually created. This could lead to undefined behavior or resource leaks. Either properly set sch->tId = threadId before calling freeSchedulerProfile, or join/detach the threadId before the test ends.

Suggested change

ASSERT_EQ(0, pthread_create(&threadId, NULL, DummySchedulerThread, NULL));

ASSERT_EQ(0, pthread_create(&threadId, NULL, DummySchedulerThread, NULL));

sch->tId = threadId;

Copilot · 2026-01-20T22:07:51Z

source/test/scheduler/schedulerTest.cpp

+    char timeRef[] = "2022-12-20T11:05:56Z";
+    // Call via function pointer
+    unsigned int result = fn(timeRef);
+    EXPECT_GE(result, 0u);


Redundant assertion. The assertion EXPECT_GE(result, 0u) is always true for an unsigned int, as unsigned integers cannot be negative by definition. This assertion provides no value and should be removed.

Suggested change

EXPECT_GE(result, 0u);

Copilot · 2026-01-20T22:07:51Z

source/test/scheduler/schedulerTest.cpp

+    // Make time1->tv_nsec - time2->tv_nsec > 1,000,000,000
+    time1.tv_sec = 10;
+    time1.tv_nsec = 200000000; // 2.1 seconds in nanoseconds
+    time2.tv_sec = 10;
+    time2.tv_nsec = 100000000;     // 0.001 seconds in nanoseconds


The comment at line 168 is inaccurate. The value 200000000 nanoseconds equals 0.2 seconds, not 2.1 seconds. For reference, 1 second = 1,000,000,000 nanoseconds.

Suggested change

// Make time1->tv_nsec - time2->tv_nsec > 1,000,000,000

time1.tv_sec = 10;

time1.tv_nsec = 200000000; // 2.1 seconds in nanoseconds

time2.tv_sec = 10;

time2.tv_nsec = 100000000; // 0.001 seconds in nanoseconds

// Make time1->tv_nsec greater than time2->tv_nsec to exercise the carry/borrow logic

time1.tv_sec = 10;

time1.tv_nsec = 200000000; // 0.2 seconds in nanoseconds

time2.tv_sec = 10;

time2.tv_nsec = 100000000; // 0.1 seconds in nanoseconds

Copilot · 2026-01-20T22:07:51Z

source/test/scheduler/schedulerTest.cpp

+    SchedulerProfile *tProfile = (SchedulerProfile *)malloc(sizeof(SchedulerProfile));
+    tProfile->name = strdup("RDKB_Profile");
+    tProfile->timeRefinSec = 0;
+    tProfile->timeRef = NULL; 


Trailing whitespace after the NULL assignment. This line has unnecessary trailing whitespace that should be removed for code cleanliness.

Suggested change

tProfile->timeRef = NULL;

tProfile->timeRef = NULL;

rosemarybennyy requested a review from a team as a code owner January 20, 2026 12:02

Copilot AI review requested due to automatic review settings January 20, 2026 12:02

Copilot started reviewing on behalf of rosemarybennyy January 20, 2026 12:02 View session

Copilot AI reviewed Jan 20, 2026

View reviewed changes

rosemarybennyy force-pushed the topic/RDK-60519 branch from b7ee61c to e91d02b Compare January 20, 2026 12:09

Copilot AI review requested due to automatic review settings January 20, 2026 12:32

rosemarybennyy force-pushed the topic/RDK-60519 branch from e91d02b to 14d2460 Compare January 20, 2026 12:32

Copilot started reviewing on behalf of rosemarybennyy January 20, 2026 12:33 View session

Copilot AI reviewed Jan 20, 2026

View reviewed changes

RDK-60519: Adding L1 unit test cases for scheduler

fcf7e4c

Reason for change: Adding L1 unit test cases for scheduler Test Procedure: Tested and verified Risks: Medium Priority: P1 Signed-off-by: Rose Mary Benny <RoseMary_Benny@comcast.com>

rosemarybennyy force-pushed the topic/RDK-60519 branch from 14d2460 to fcf7e4c Compare January 20, 2026 15:17

Merge branch 'develop' into topic/RDK-60519

05ea0b6

Copilot AI review requested due to automatic review settings January 20, 2026 22:03

Copilot started reviewing on behalf of shibu-kv January 20, 2026 22:04 View session

Copilot AI reviewed Jan 20, 2026

View reviewed changes

shibu-kv approved these changes Jan 20, 2026

View reviewed changes

shibu-kv merged commit c8d2a07 into develop Jan 20, 2026
21 checks passed

github-actions bot locked and limited conversation to collaborators Jan 20, 2026

		{
		initScheduler((TimeoutNotificationCB)ReportProfiles_ToutCb, (ActivationTimeoutCB)ReportProfiles_ActivationToutCb, (NotifySchedulerstartCB)NotifySchedulerstartCb);

	ASSERT_EQ(0, pthread_create(&threadId, NULL, DummySchedulerThread, NULL));
	ASSERT_EQ(0, pthread_create(&threadId, NULL, DummySchedulerThread, NULL));

	ASSERT_EQ(0, pthread_create(&threadId, NULL, DummySchedulerThread, NULL));
	ASSERT_EQ(0, pthread_create(&threadId, NULL, DummySchedulerThread, NULL));
	sch->tId = threadId;

RDK-60519: Adding L1 unit test cases for scheduler #238

RDK-60519: Adding L1 unit test cases for scheduler #238

Conversation

rosemarybennyy commented Jan 20, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants