Clean up the tests

Author: nivi-apple

src/darwin/Framework/CHIP/MTRDeviceControllerDataStore.mm

@@ -475,6 +475,7 @@ - (BOOL)_deleteEndpointIndexForNodeID:(NSNumber *)nodeID
         return NO;
     }
 
+
     return [self _removeAttributeCacheValueForKey:[self _endpointIndexKeyForNodeID:nodeID]];
 }
 
@@ -559,6 +560,7 @@ - (BOOL)_deleteClusterDataForNodeID:(NSNumber *)nodeID endpointID:(NSNumber *)en
         MTR_LOG_ERROR("%s: unexpected nil input", __func__);
         return NO;
     }
+
     return [self _removeAttributeCacheValueForKey:[self _clusterDataKeyForNodeID:nodeID endpointID:endpointID clusterID:clusterID]];
 }
 
@@ -749,7 +751,11 @@ - (void)removeAttributes:(NSSet<NSNumber *> *)attributes fromCluster:(MTRCluster
     for (NSNumber * attribute in attributes) {
         [clusterData removeValueForAttribute:attribute];
     }
-    [self _storeClusterData:clusterData forNodeID:nodeID endpointID:path.endpoint clusterID:path.cluster];
+    BOOL success = [self _storeClusterData:clusterData forNodeID:nodeID endpointID:path.endpoint clusterID:path.cluster];
+    if (!success) {
+        MTR_LOG_ERROR("removeAttributes: _storeClusterData failed for node 0x%016llX endpoint %u", nodeID.unsignedLongLongValue, path.endpoint.unsignedShortValue);
+    }
+    MTR_LOG("removeAttributes: Deleted attributes %@ from endpoint %u cluster 0x%08lX for node 0x%016llX successfully", attributes, path.endpoint.unsignedShortValue, path.cluster.unsignedLongValue, nodeID.unsignedLongLongValue);
 }
 
 - (void)clearAllStoredClusterData

src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m

@@ -2278,6 +2278,7 @@ - (void)test011_testDataStorageUpdatesWhenRemovingEndpoints
         // just checking data storage should suffice here.
         dispatch_sync(self->_storageQueue, ^{
             XCTAssertTrue([[controller.controllerDataStore _fetchEndpointIndexForNodeID:deviceID] isEqualToArray:testEndpoints]);
+
             // Populate the initialClusterIndex to use as a reference for all cluster paths later.
             for (NSNumber * endpoint in testEndpoints) {
                 [initialClusterIndex setObject:[controller.controllerDataStore _fetchClusterIndexForNodeID:deviceID endpointID:endpoint] forKey:endpoint];
@@ -2300,7 +2301,7 @@ - (void)test011_testDataStorageUpdatesWhenRemovingEndpoints
     // Inject a fake attribute report deleting endpoint 2 from the parts list at the root endpoint.
     dataVersionForPartsList = [NSNumber numberWithUnsignedLongLong:(dataVersionForPartsList.unsignedLongLongValue + 1)];
 
-    // Delete the last endpoint from the attribute value in parts list.
+    // Delete endpoint 2 from the attribute value in parts list.
     NSNumber * toBeDeletedEndpoint = @2;
     __block id endpointData =
         @{
@@ -2330,7 +2331,8 @@ - (void)test011_testDataStorageUpdatesWhenRemovingEndpoints
         for (NSDictionary<NSString *, id> * attributeDict in attributeReport) {
             MTRAttributePath * attributePath = attributeDict[MTRAttributePathKey];
             XCTAssertNotNil(attributePath);
-            // Get the new updated parts list value to get the test endpoints.
+
+            // Get the new updated parts list value to get the new test endpoints.
             if ([attributePath.endpoint isEqualToNumber:rootEndpoint] && attributePath.cluster.unsignedLongValue == MTRClusterIDTypeDescriptorID && attributePath.attribute.unsignedLongValue == MTRAttributeIDTypeClusterDescriptorAttributePartsListID) {
                 testDataForPartsList = attributeDict[MTRDataKey];
                 XCTAssertNotNil(testDataForPartsList);
@@ -2346,7 +2348,7 @@ - (void)test011_testDataStorageUpdatesWhenRemovingEndpoints
         XCTAssertNotNil(testClusterDataValueForPartsList);
         testEndpoints = [self getEndpointArrayFromPartsList:testDataForPartsList forDevice:device];
 
-        // Make sure that the cluster data in the data storage for the cluster for endpoints 0 and 1 are present but not for endpoint 2.
+        // Make sure that the cluster data in the data storage for endpoints 0 and 1 are present but not for endpoint 2.
         // We do not need to check _persistedClusterData here. _persistedClusterData will be paged in from storage when needed so
         // just checking data storage should suffice here.
         dispatch_sync(self->_storageQueue, ^{
@@ -2398,11 +2400,10 @@ - (void)test012_testDataStorageUpdatesWhenRemovingClusters
 
     // This test will do the following -
     // 1. Get the data version and attribute value of the server list for endpoint 1 to inject a fake report. The attribute report will delete cluster ID - MTRClusterIDTypeIdentifyID.
-    //    That should cause the cluster to be removed from cluster index and cluster data for that cluster to be removed from data storage.
-    // 2. The data store is populated with MTRClusterIDTypeIdentifyID in the cluster index and cluster data initially.
-    //    Also _persistedClusters and _persistedClusterData has the cluster path and cluster data for cluster ID - MTRClusterIDTypeIdentifyID.
+    //    That should cause the cluster to be removed from cluster index for endpoint 1 and the cluster data for the removed cluster should be cleared from data storage.
+    // 2. The data store is populated with MTRClusterIDTypeIdentifyID in the cluster index and cluster data for endpoint 1 initially.
     // 3. After the fake attribute report is injected with deleted cluster ID - MTRClusterIDTypeIdentifyID, make sure the data store is still populated with cluster index and
-    //    cluster data for other clusters at endpoint 1 but not the deleted cluster.
+    //    cluster data for all other clusters at endpoint 1 but not the deleted cluster.
     __block NSMutableArray * testClusterDataValue;
     delegate.onAttributeDataReceived = ^(NSArray<NSDictionary<NSString *, id> *> * attributeReport) {
         attributeReportsReceived += attributeReport.count;
@@ -2438,7 +2439,7 @@ - (void)test012_testDataStorageUpdatesWhenRemovingClusters
         XCTAssertNotNil(testClusterDataValue);
 
         // Make sure that the cluster data in the data storage has cluster ID - MTRClusterIDTypeIdentifyID in the cluster index for endpoint 1
-        // and cluster data for MTRClusterIDTypeIdentifyID exists
+        // and cluster data for MTRClusterIDTypeIdentifyID exists.
         // We do not need to check _persistedClusterData here. _persistedClusterData will be paged in from storage when needed so
         // just checking data storage should suffice here.
         dispatch_sync(self->_storageQueue, ^{
@@ -2477,6 +2478,7 @@ - (void)test012_testDataStorageUpdatesWhenRemovingClusters
     };
 
     delegate.onReportEnd = ^{
+
         // Make sure that the cluster data does not have cluster ID - MTRClusterIDTypeIdentifyID in the cluster index for endpoint 1
         // and cluster data for MTRClusterIDTypeIdentifyID is nil.
         // We do not need to check _persistedClusterData here. _persistedClusterData will be paged in from storage when needed so
@@ -2528,9 +2530,9 @@ - (void)test013_testDataStorageUpdatesWhenRemovingAttributes
     __block NSMutableArray * testClusterDataValue;
 
     // This test will do the following -
-    // 1. Get the data version and attribute value of the attribute list for endpoint 1 to inject a fake report with attribute 1 removed.
+    // 1. Get the data version and attribute value of the attribute list for endpoint 1 to inject a fake report with attribute 1 removed from MTRClusterIDTypeIdentifyID.
     // 2. The data store is populated with cluster data for MTRClusterIDTypeIdentifyID cluster and has all attributes including attribute 1.
-    // 3. After the fake attribute report is injected, make sure the data store is populated with cluster data for MTRClusterIDTypeIdentifyID
+    // 3. After the fake attribute report is injected, make sure the data store is populated with cluster data for all attributes in MTRClusterIDTypeIdentifyID
     //    cluster except for attribute 1 which has been deleted.
     delegate.onAttributeDataReceived = ^(NSArray<NSDictionary<NSString *, id> *> * attributeReport) {
         attributeReportsReceived += attributeReport.count;
@@ -2565,7 +2567,7 @@ - (void)test013_testDataStorageUpdatesWhenRemovingAttributes
 
                 // Make sure that the cluster data in the data storage is populated with cluster data for MTRClusterIDTypeIdentifyID cluster
                 // and has all attributes including attribute 1.
-                // We will be paged in the cluster data from storage to check the above.
+                // We will page in the cluster data from storage to check the above.
                 MTRClusterPath * path = [MTRClusterPath clusterPathWithEndpointID:testEndpoint clusterID:cluster];
 
                 if ([cluster isEqualToNumber:@(MTRClusterIDTypeIdentifyID)]) {
@@ -2615,9 +2617,10 @@ - (void)test013_testDataStorageUpdatesWhenRemovingAttributes
     };
 
     delegate.onReportEnd = ^{
+
         // Make sure that the cluster data in the data storage is populated with cluster data for MTRClusterIDTypeIdentifyID cluster
         // and has all attributes except attribute 1 which was deleted.
-        // We will be paged in the cluster data from storage to check the above.
+        // We will page in the cluster data from storage to check the above.
         dispatch_sync(self->_storageQueue, ^{
             initialClusterIndex = [[controller.controllerDataStore _fetchClusterIndexForNodeID:deviceID endpointID:testEndpoint] mutableCopy];
             XCTAssertNotNil(initialClusterIndex);