Unit tests for NetworkCommissioning cluster using Ethernet and bugfixes. (project-chip#42189)

* Add unit tests for `NetworkCommisioning` cluster for ethernet

* Bug fixes

`Init` was called on the cluster then instead `Deinit` was called `Shutdown`

Write attribute on InterfaceEnabled with value of false when not supported didn't return INVALID_ACTION status code

* Restyled by whitespace

* Restyled by clang-format

* Restyled by gn

* Resolving reviews and suggestions

---------

Co-authored-by: Restyled.io <[email protected]>

Author: Hayk10002

src/app/clusters/network-commissioning/CodegenInstance.cpp

@@ -46,7 +46,7 @@ CHIP_ERROR Instance::Init()
 void Instance::Shutdown()
 {
     TEMPORARY_RETURN_IGNORED CodegenDataModelProvider::Instance().Registry().Unregister(&mCluster.Cluster());
-    mCluster.Cluster().Shutdown();
+    mCluster.Cluster().Deinit();
 }
 
 } // namespace NetworkCommissioning

src/app/clusters/network-commissioning/NetworkCommissioningCluster.cpp

@@ -261,7 +261,7 @@ void NetworkCommissioningCluster::SetLastConnectErrorValue(Attributes::LastConne
 {
     if (mLastConnectErrorValue.Update(connectErrorValue))
     {
-        NotifyAttributeChanged(Attributes::LastConnectErrorValue::TypeInfo::GetAttributeId());
+        NotifyAttributeChanged(Attributes::LastConnectErrorValue::Id);
     }
 }
 
@@ -273,12 +273,12 @@ void NetworkCommissioningCluster::SetLastNetworkId(ByteSpan lastNetworkId)
 
     memcpy(mLastNetworkID, lastNetworkId.data(), lastNetworkId.size());
     mLastNetworkIDLen = static_cast<uint8_t>(lastNetworkId.size());
-    NotifyAttributeChanged(Attributes::LastNetworkID::TypeInfo::GetAttributeId());
+    NotifyAttributeChanged(Attributes::LastNetworkID::Id);
 }
 
 void NetworkCommissioningCluster::ReportNetworksListChanged()
 {
-    NotifyAttributeChanged(Attributes::Networks::TypeInfo::GetAttributeId());
+    NotifyAttributeChanged(Attributes::Networks::Id);
 }
 
 void NetworkCommissioningCluster::OnNetworkingStatusChange(Status aCommissioningError, Optional<ByteSpan> aNetworkId,
@@ -1081,10 +1081,18 @@ DataModel::ActionReturnStatus NetworkCommissioningCluster::WriteAttribute(const
     {
         bool value;
         ReturnErrorOnFailure(decoder.Decode(value));
-        return NotifyAttributeChangedIfSuccess(request.path.mAttributeId, SetInterfaceEnabled(value));
+        CHIP_ERROR err = SetInterfaceEnabled(value);
+
+        // Spec. 11.9.6.5 -- "If not supported, a write to this attribute with a value of false SHALL fail with a status of
+        // INVALID_ACTION."
+        if (err == CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE)
+        {
+            return Protocols::InteractionModel::Status::InvalidAction;
+        }
+        return NotifyAttributeChangedIfSuccess(request.path.mAttributeId, err);
     }
 
-    return Protocols::InteractionModel::Status::InvalidAction;
+    return Protocols::InteractionModel::Status::UnsupportedWrite;
 }
 
 std::optional<DataModel::ActionReturnStatus> NetworkCommissioningCluster::InvokeCommand(const DataModel::InvokeRequest & request,

src/app/clusters/network-commissioning/tests/BUILD.gn

@@ -23,11 +23,12 @@ chip_test_suite("tests") {
 
   test_sources = [
     "TestNetworkCommissioningCluster.cpp",
+    "TestNetworkCommissioningClusterEthernet.cpp",
     "TestThreadResponse.cpp",
     "TestWifiResponse.cpp",
   ]
 
-  sources = [ "FakeWifiDriver.h" ]
+  sources = [ "FakeDrivers.h" ]
 
   cflags = [ "-Wconversion" ]
 

src/app/clusters/network-commissioning/tests/FakeWifiDriver.h renamed to src/app/clusters/network-commissioning/tests/FakeDrivers.h

@@ -13,14 +13,109 @@
  *    See the License for the specific language governing permissions and
  *    limitations under the License.
  */
-#include "clusters/NetworkCommissioning/Enums.h"
-#include "lib/core/CHIPError.h"
+#include <clusters/NetworkCommissioning/Attributes.h>
+#include <clusters/NetworkCommissioning/Enums.h>
+#include <lib/core/CHIPError.h>
+#include <lib/support/Span.h>
 #include <platform/NetworkCommissioning.h>
 #include <protocols/interaction_model/StatusCode.h>
 
 namespace chip {
 namespace Testing {
 
+class FakeEthernetDriver : public DeviceLayer::NetworkCommissioning::EthernetDriver
+{
+public:
+    class FakeNetworkIterator : public DeviceLayer::NetworkCommissioning::NetworkIterator
+    {
+    public:
+        void Set(Span<DeviceLayer::NetworkCommissioning::Network> networks)
+        {
+            mNetworks    = networks;
+            currentindex = 0;
+        }
+        bool Next(DeviceLayer::NetworkCommissioning::Network & item) override
+        {
+            VerifyOrReturnValue(currentindex < mNetworks.size(), false);
+            item = mNetworks[currentindex++];
+            return true;
+        }
+        size_t Count() override { return mNetworks.size(); }
+        void Release() override {}
+
+    private:
+        Span<DeviceLayer::NetworkCommissioning::Network> mNetworks;
+        size_t currentindex = 0;
+    };
+
+    CHIP_ERROR Init(NetworkStatusChangeCallback * networkStatusChangeCallback) override
+    {
+        mNetworkStatusChangeCallback = networkStatusChangeCallback;
+        return CHIP_NO_ERROR;
+    }
+
+    uint8_t GetMaxNetworks() override { return 1; };
+
+    void SetNetwork(ByteSpan interfaceName)
+    {
+        auto span = MutableByteSpan{ mNetwork.networkID, sizeof(mNetwork.networkID) };
+        SuccessOrDie(CopySpanToMutableSpan(interfaceName, span));
+        mNetwork.networkIDLen = static_cast<uint8_t>(interfaceName.size());
+        mNetwork.connected    = true;
+        mNetworkCount         = 1;
+
+        mNetworkStatusChangeCallback->OnNetworkingStatusChange(
+            app::Clusters::NetworkCommissioning::NetworkCommissioningStatusEnum::kSuccess, Optional{ interfaceName },
+            testErrorValue);
+    }
+
+    void SetNetworkConnected(bool connected) { mNetwork.connected = connected; }
+
+    void SetNoNetwork() { mNetworkCount = 0; }
+
+    DeviceLayer::NetworkCommissioning::NetworkIterator * GetNetworks() override
+    {
+        if (mNetworkCount == 0)
+        {
+            mNetworkIterator.Set({});
+        }
+        else
+        {
+            mNetworkIterator.Set({ &mNetwork, 1 });
+        }
+
+        return &mNetworkIterator;
+    };
+
+    void EnableDisabling(bool enabledAllowed) { mSetEnabledAllowed = enabledAllowed; }
+
+    CHIP_ERROR SetEnabled(bool enabled) override
+    {
+        if (GetEnabled() == enabled)
+        {
+            return CHIP_NO_ERROR;
+        }
+        if (mSetEnabledAllowed)
+        {
+            mEnabled = enabled;
+            return CHIP_NO_ERROR;
+        }
+        return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE;
+    }
+
+    bool GetEnabled() override { return mEnabled; };
+
+    Optional<int32_t> testErrorValue{ 123 };
+
+private:
+    NetworkStatusChangeCallback * mNetworkStatusChangeCallback;
+    FakeNetworkIterator mNetworkIterator;
+    DeviceLayer::NetworkCommissioning::Network mNetwork;
+    size_t mNetworkCount    = 0;
+    bool mSetEnabledAllowed = false;
+    bool mEnabled           = true;
+};
+
 class FakeWiFiDriver : public DeviceLayer::NetworkCommissioning::WiFiDriver
 {
 public:

src/app/clusters/network-commissioning/tests/TestNetworkCommissioningCluster.cpp

@@ -36,7 +36,7 @@
 #include <lib/support/ReadOnlyBuffer.h>
 #include <platform/NetworkCommissioning.h>
 
-#include "FakeWifiDriver.h"
+#include "FakeDrivers.h"
 
 namespace {
 
@@ -114,7 +114,7 @@ TEST_F(TestNetworkCommissioningCluster, TestNotifyOnEnableInterface)
     {
         WriteOperation writeOp(kRootEndpointId, NetworkCommissioning::Id, InterfaceEnabled::Id);
         writeOp.SetSubjectDescriptor(kAdminSubjectDescriptor);
-        AttributeValueDecoder decoder = writeOp.DecoderFor(true);
+        AttributeValueDecoder decoder = writeOp.DecoderFor(false);
 
         // no notification if enable fails
         context.ChangeListener().DirtyList().clear();