VPAAMP-946: Fix the Logs for the Updated Config Values

[srikanthreddybijjam-comcast]

Reason for change: Modified the config update logs with accurate value
Test Procedure: Refer the ticket VPAAMP-946
Priority: P2

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Vinish100]

Check if there is a duplicate L1 for the same case. For validation of new logs, lets use a simple L2

[Vinish100]

Looks good to me 👍

[Copilot]

Pull request overview

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

Suppressed comments (4)

test/utests/tests/ConfigTests/ConfigTests.cpp:1523

• This int-owner block only covers the rejected path and uses cJSON_Parse without asserting success. Adding an accepted case (APPLICATION set followed by DEV_CFG JSON set) makes the owner semantics covered for ints and improves L1 test validity.

	// Int (abrCacheLife): dev-cfg sets 60, app tries to set 5 -> retained as 60
	mAampConfig->SetConfigValue(AAMP_DEV_CFG_SETTING, eAAMPConfig_ABRCacheLife, 60);
	cJSON *abrCacheLife5 = cJSON_Parse(R"({"abrCacheLife":5})");
	mAampConfig->ProcessConfigJson(abrCacheLife5, AAMP_APPLICATION_SETTING);
	cJSON_Delete(abrCacheLife5);

test/utests/tests/ConfigTests/ConfigTests.cpp:1515

• This section only covers the rejected float-owner path and doesn’t assert the accepted path, despite the test intent comment. Also, cJSON_Parse is used without asserting success. Covering accepted+rejected (and asserting parse success) aligns the test oracle with the stated intent and improves L1 validity.

	// Float (manifestTimeout): dev-cfg sets 30s, app tries to set 5s -> retained as 30s
	mAampConfig->SetConfigValue(AAMP_DEV_CFG_SETTING, eAAMPConfig_ManifestTimeout, 30.0);
	cJSON *manifestTimeout5s = cJSON_Parse(R"({"manifestTimeout":5})");
	mAampConfig->ProcessConfigJson(manifestTimeout5s, AAMP_APPLICATION_SETTING);
	cJSON_Delete(manifestTimeout5s);

test/utests/tests/ConfigTests/ConfigTests.cpp:1492

• The new test name/comment says it verifies logging ("LogsActualValue"), but the test only asserts stored config values and does not validate log output. This is misleading for future readers; either capture/assert logs, or rename/update the comment to match what is actually tested.

// Verifies that ProcessConfigJson logs and stores the actual in-effect value for all config types:
// rejected sets (lower-priority owner) retain the higher-priority value;
// accepted sets (higher-priority owner) update the value.
TEST_F(AampConfigTests, ProcessConfigJson_LogsActualValueAfterOwnerChange)
{

test/utests/tests/ConfigTests/ConfigTests.cpp:1510

• The "accepted bool set" section doesn’t actually exercise an accepted owner override: after setting DEV_CFG to true, the subsequent APPLICATION SetConfigValue(false) is rejected (lower priority), so the value never becomes false and the later JSON set doesn’t prove the update path. Reorder the steps so APPLICATION sets first and DEV_CFG updates via ProcessConfigJson, then verify APPLICATION rejection.

This issue also appears in the following locations of the same file:

• line 1511
• line 1519

	// Bool (bulkTimedMetadata): dev-cfg sets true, app tries to set false -> retained as true
	mAampConfig->SetConfigValue(AAMP_DEV_CFG_SETTING, eAAMPConfig_BulkTimedMetaReport, true);
	cJSON *bulkTimedMetaFalse = cJSON_Parse(R"({"bulkTimedMetadata":false})");
	mAampConfig->ProcessConfigJson(bulkTimedMetaFalse, AAMP_APPLICATION_SETTING);
	cJSON_Delete(bulkTimedMetaFalse);