RDKCOM-5487: RDKBWIFI-206: add mocks, build, and limited platform ind… by pradeeptakdas · Pull Request #1300 · rdkcentral/OneWifi

pradeeptakdas · 2026-07-27T18:46:59Z

…ependent L1 tests (#702)

RDKBWIFI-206: add mocks and build for platform independent L1 tests

on-behalf-of: @permanence-ai github-ai@permanence.ai

RDKBWIFI-206: add unittests for wifi_ctrl_webconfig.c

on-behalf-of: @permanence-ai github-ai@permanence.ai

remove symbols that are no longer necessary to mock

on-behalf-of: @permanence-ai github-ai@permanence.ai

update mockplatform/makefile to match recent updates
remove wifi_sensing from the Makefile again to work with latest changes
more explicitly use the rpi setup.sh script as the intent is to be identical
minor naming conventions and typo updates
update mockplatform makefile for multi_ap includes
Signed-off-by: Fritz Heckel fwph@fwph.net

…ependent L1 tests (rdkcentral#702) * RDKBWIFI-206: add mocks and build for platform independent L1 tests on-behalf-of: @permanence-ai <github-ai@permanence.ai> * RDKBWIFI-206: add unittests for wifi_ctrl_webconfig.c on-behalf-of: @permanence-ai <github-ai@permanence.ai> * remove symbols that are no longer necessary to mock on-behalf-of: @permanence-ai <github-ai@permanence.ai> * update mockplatform/makefile to match recent updates * remove wifi_sensing from the Makefile again to work with latest changes * more explicitly use the rpi setup.sh script as the intent is to be identical * minor naming conventions and typo updates * update mockplatform makefile for multi_ap includes

Copilot

Pull request overview

Adds a “mockplatform” build target to enable limited, platform-independent(ish) L1/unit testing and introduces initial gtests around wifi_ctrl_webconfig.c apply-path functions, along with platform stub implementations to satisfy link dependencies in CI.

Changes:

Added new gtest suite covering null-pointer safety and basic empty-map paths for several webconfig_*_apply functions.
Introduced mockplatform Linux makefile target that builds and runs gtests (including a small platform-mock shim).
Updated GitHub Actions workflow matrix to build both the existing Raspberry Pi target and the new mockplatform unit-test target (and install required packages).

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`source/test/wifi_ctrl_webconfig_test.cpp`	New gtests for `webconfig_*_apply` functions (null checks + empty/same-map scenarios).
`source/test/platform_mocks.c`	Adds stubbed platform/HAL functions needed to link the unit-test binary.
`source/test/gtest_main.cpp`	Whitespace-only header formatting adjustment.
`build/linux/mockplatform/makefile`	New build target to compile/link/run gtests, including `wifi_mgr` main-symbol stripping.
`.github/workflows/makefile.yml`	CI matrix expanded to include mockplatform unit-test build; installs gtest/gmock deps.

Comments suppressed due to low confidence (12)

source/test/wifi_ctrl_webconfig_test.cpp:98

Typo in comment: "butuse" → "but use".

    // expected that RETURN_ERR is -1, butuse the macro if possible

source/test/wifi_ctrl_webconfig_test.cpp:140

Typo in comment: "butuse" → "but use".

    // expected that RETURN_ERR is -1, butuse the macro if possible

source/test/wifi_ctrl_webconfig_test.cpp:146

Typo in comment: "butuse" → "but use".

    // expected that RETURN_ERR is -1, butuse the macro if possible

source/test/wifi_ctrl_webconfig_test.cpp:188

Typo in comment: "butuse" → "but use".

    // expected that RETURN_ERR is -1, butuse the macro if possible

source/test/wifi_ctrl_webconfig_test.cpp:194

Typo in comment: "butuse" → "but use".

    // expected that RETURN_ERR is -1, butuse the macro if possible

source/test/wifi_ctrl_webconfig_test.cpp:113

ASSERT_EQ aborts the test on failure and can skip the wifi_mgr cleanup/reset that follows, polluting subsequent tests that use the singleton wifi_mgr. Prefer EXPECT_EQ so cleanup still runs.

    ASSERT_EQ(webconfig_steering_config_apply(NULL, &data), RETURN_OK);

source/test/wifi_ctrl_webconfig_test.cpp:126

If this ASSERT_EQ fails, the cleanup that destroys/resets mgr->steering_config_map won't run, which can impact later tests sharing the wifi_mgr singleton. Prefer EXPECT_EQ so cleanup still executes.

    ASSERT_EQ(webconfig_steering_config_apply(NULL, &data), RETURN_OK);

source/test/wifi_ctrl_webconfig_test.cpp:161

If this ASSERT_EQ fails, the subsequent line that resets mgr->steering_client_map won't run, potentially breaking later tests that reuse wifi_mgr. Prefer EXPECT_EQ so cleanup still runs.

    ASSERT_EQ(webconfig_steering_clients_apply(NULL, &data), RETURN_OK);

source/test/wifi_ctrl_webconfig_test.cpp:174

ASSERT_EQ aborts the test on failure and will skip hash_map_destroy/mgr reset below, which can pollute subsequent tests. Prefer EXPECT_EQ so cleanup still executes.

    ASSERT_EQ(webconfig_steering_clients_apply(NULL, &data), RETURN_OK);

source/test/wifi_ctrl_webconfig_test.cpp:209

ASSERT_EQ aborts the test on failure and can skip resetting mgr->stats_config_map below, leaving the singleton wifi_mgr in a bad state for subsequent tests. Prefer EXPECT_EQ so cleanup still runs.

    ASSERT_EQ(webconfig_stats_config_apply(NULL, &data), RETURN_OK);

source/test/wifi_ctrl_webconfig_test.cpp:222

If this ASSERT_EQ fails, the cleanup that destroys/resets mgr->stats_config_map won't run, affecting later tests sharing the singleton wifi_mgr. Prefer EXPECT_EQ so cleanup still runs.

    ASSERT_EQ(webconfig_stats_config_apply(NULL, &data), RETURN_OK);

source/test/wifi_ctrl_webconfig_test.cpp:214

Test name says "SameStatsConfigMap", but this test creates different maps for mgr->stats_config_map and data.stats_config_map. Rename so the test name reflects the scenario under test.

TEST(WifiCtrlWebconfig, StatsConfigSameStatsConfigMap)

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

+clean:
+	$(RM) $(ALLOBJECTS) $(ALL_CMN_LIB_OBJECTS) $(ALL_HAL_LIB_OBJECTS) $(CMN_LIBRARY) $(HE_BUS_LIBRARY) $(WEBCONFIG_LIBRARY) $(HAL_LIBRARY) $(HOSTAP_LIBRARY) $(PROGRAM) $(GTEST_BINARY)


+TEST(WifiCtrlWebconfig, SteeringConfigApplyNullArguments)
+{
+    ASSERT_EXIT((webconfig_steering_config_apply(NULL, NULL), exit(0)), ::testing::ExitedWithCode(0), ".*");
+    // expected that RETURN_ERR is -1, butuse the macro if possible


+    wifi_mgr_t *mgr = get_wifimgr_obj();
+    mgr->vif_neighbors_map = hash_map_create();
+
+    ASSERT_EQ(webconfig_vif_neighbors_apply(NULL, &data), RETURN_OK);


+    mgr->steering_client_map = NULL;
+}
+
+TEST(WifiCtrlWebconfig, SteeringClientsSameClientMap)


+    ASSERT_EQ(webconfig_stats_config_apply(NULL, NULL), RETURN_ERR);
+}
+
+TEST(WifiCtrlWebconfig, StatsConfigsApplyNullStatsConfigMap)


Copilot

Pull request overview

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

Comments suppressed due to low confidence (8)

source/test/wifi_ctrl_webconfig_test.cpp:93

Same issue as above: this test calls webconfig_steering_config_apply(NULL, NULL) directly in the parent process. In the current implementation the data pointer is dereferenced without a NULL guard, which can segfault the entire suite. Prefer checking return value inside ASSERT_EXIT to isolate crashes.

    ASSERT_EXIT((webconfig_steering_config_apply(NULL, NULL), exit(0)), ::testing::ExitedWithCode(0), ".*");
    // expected that RETURN_ERR is -1, butuse the macro if possible
    ASSERT_EQ(webconfig_steering_config_apply(NULL, NULL), RETURN_ERR);

source/test/wifi_ctrl_webconfig_test.cpp:141

This test calls webconfig_steering_clients_apply(NULL, NULL) directly in the parent process. Since the implementation dereferences data without a NULL check, this risks a SIGSEGV that aborts the whole test binary. Prefer asserting the return inside ASSERT_EXIT so crashes are contained.

    ASSERT_EXIT((webconfig_steering_clients_apply(NULL, NULL), exit(0)), ::testing::ExitedWithCode(0), ".*");
    // expected that RETURN_ERR is -1, butuse the macro if possible
    ASSERT_EQ(webconfig_steering_clients_apply(NULL, NULL), RETURN_ERR);

source/test/wifi_ctrl_webconfig_test.cpp:189

This test calls webconfig_stats_config_apply(NULL, NULL) directly in the parent process. The implementation currently dereferences data without a NULL guard, so this can segfault and terminate the entire suite. Prefer checking the return value inside ASSERT_EXIT to keep failures isolated.

    ASSERT_EXIT((webconfig_stats_config_apply(NULL, NULL), exit(0)), ::testing::ExitedWithCode(0), ".*");
    // expected that RETURN_ERR is -1, butuse the macro if possible
    ASSERT_EQ(webconfig_stats_config_apply(NULL, NULL), RETURN_ERR);

source/test/wifi_ctrl_webconfig_test.cpp:166

Test name says “SameClientMap”, but the test initializes data.steering_client_map and mgr->steering_client_map with two different hash maps. Renaming the test avoids confusion when diagnosing failures.

TEST(WifiCtrlWebconfig, SteeringClientsSameClientMap)

source/test/wifi_ctrl_webconfig_test.cpp:192

Minor naming consistency: this test name uses “StatsConfigs” (plural) while the surrounding tests use “StatsConfig”.

TEST(WifiCtrlWebconfig, StatsConfigsApplyNullStatsConfigMap)

source/test/wifi_ctrl_webconfig_test.cpp:98

Typo in comment: “butuse” → “but use”.

    // expected that RETURN_ERR is -1, butuse the macro if possible

source/test/wifi_ctrl_webconfig_test.cpp:146

Typo in comment: “butuse” → “but use”.

    // expected that RETURN_ERR is -1, butuse the macro if possible

source/test/wifi_ctrl_webconfig_test.cpp:194

Typo in comment: “butuse” → “but use”.

    // expected that RETURN_ERR is -1, butuse the macro if possible

+    ASSERT_EXIT((webconfig_vif_neighbors_apply(NULL, NULL), exit(0)), ::testing::ExitedWithCode(0), ".*");
+    // expected that RETURN_ERR is -1, but use the macro if possible
+    ASSERT_EQ(webconfig_vif_neighbors_apply(NULL, NULL), RETURN_ERR);


+    mgr->stats_config_map = NULL;
+}
+
+TEST(WifiCtrlWebconfig, StatsConfigSameStatsConfigMap)


mateuszCieslak-GL

Seriously? Rehash of #1296 under different account.
Marking as blocked for wider review,
Permanence AI (on which agentic's tool behalf this was created) ceased to exist last month.
I dont think we can proceed with this, without legal review (i.e. CLA agreement) and/or manager who can vouch for the process used.

Copilot AI review requested due to automatic review settings July 27, 2026 18:47

github-actions Bot added the community contribution Contributions from community. label Jul 27, 2026

Copilot started reviewing on behalf of pradeeptakdas July 27, 2026 18:47 View session

Update makefile.yml

96ad5e9

Copilot AI reviewed Jul 27, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 27, 2026 18:52

Copilot started reviewing on behalf of pradeeptakdas July 27, 2026 18:54 View session

Copilot AI reviewed Jul 27, 2026

View reviewed changes

mateuszCieslak-GL requested changes Jul 27, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

RDKCOM-5487: RDKBWIFI-206: add mocks, build, and limited platform ind… - #1300

RDKCOM-5487: RDKBWIFI-206: add mocks, build, and limited platform ind…#1300
pradeeptakdas wants to merge 2 commits into
rdkcentral:mainfrom
pradeeptakdas:RDKCOM-5487

pradeeptakdas commented Jul 27, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

mateuszCieslak-GL left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		clean:
		$(RM) $(ALLOBJECTS) $(ALL_CMN_LIB_OBJECTS) $(ALL_HAL_LIB_OBJECTS) $(CMN_LIBRARY) $(HE_BUS_LIBRARY) $(WEBCONFIG_LIBRARY) $(HAL_LIBRARY) $(HOSTAP_LIBRARY) $(PROGRAM) $(GTEST_BINARY)

Uh oh!

Conversation

pradeeptakdas commented Jul 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

mateuszCieslak-GL left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants