Add python YAML runner script to runs test over chip-tool and the pla…

…ceholder apps (#25155)

* [chip-tool] Add chip-tool adapter for running tests with matter_yamltests

* [examples/placeholder] Add placeholder adapter for running tests with matter_yamltests

* Add python YAML runner script to runs test over chip-tool and the placeholder apps

* Add chip-tool over python runs to the test suite

* Use a scoped lock to make Tsan happy

* [chip-tool] Update DiscoverCommissionablesCommand to avoid a data race when cleaning up parameters

* [chip-tool] Do not call ErrorStr after RunCommand in interactive mode as ErrorStr use a single static buffer to compute the error string and in interactive mode the stack continue to do some work and it may race

.github/workflows/tests.yaml

@@ -32,7 +32,7 @@ env:
 jobs:
     test_suites_linux:
         name: Test Suites - Linux
-        timeout-minutes: 145
+        timeout-minutes: 180
 
         strategy:
             matrix:
@@ -226,13 +226,32 @@ jobs:
                      --tv-app ./out/linux-x64-tv-app-${BUILD_VARIANT}/chip-tv-app \
                      --bridge-app ./out/linux-x64-bridge-${BUILD_VARIANT}/chip-bridge-app \
                   "
+
+            - name: Run Tests using the python parser sending commands to chip-tool
+              timeout-minutes: 65
+              run: |
+                  ./scripts/run_in_build_env.sh \
+                  "./scripts/tests/run_test_suite.py \
+                     --runner chip_tool_python \
+                     --chip-tool ./out/linux-x64-chip-tool${CHIP_TOOL_VARIANT}-${BUILD_VARIANT}/chip-tool \
+                     run \
+                     --iterations 1 \
+                     --test-timeout-seconds 120 \
+                     --all-clusters-app ./out/linux-x64-all-clusters-${BUILD_VARIANT}/chip-all-clusters-app \
+                     --lock-app ./out/linux-x64-lock-${BUILD_VARIANT}/chip-lock-app \
+                     --ota-provider-app ./out/linux-x64-ota-provider-${BUILD_VARIANT}/chip-ota-provider-app \
+                     --ota-requestor-app ./out/linux-x64-ota-requestor-${BUILD_VARIANT}/chip-ota-requestor-app \
+                     --tv-app ./out/linux-x64-tv-app-${BUILD_VARIANT}/chip-tv-app \
+                     --bridge-app ./out/linux-x64-bridge-${BUILD_VARIANT}/chip-bridge-app \
+                  "
+
             - name: Run Tests using chip-repl (skip slow)
               timeout-minutes: 45
               if: github.event_name == 'pull_request'
               run: |
                   ./scripts/run_in_build_env.sh \
                   "./scripts/tests/run_test_suite.py \
-                     --run-yamltests-with-chip-repl \
+                     --runner chip_repl_python \
                      --exclude-tags MANUAL \
                      --exclude-tags FLAKY \
                      --exclude-tags IN_DEVELOPMENT \
@@ -253,7 +272,7 @@ jobs:
               run: |
                   ./scripts/run_in_build_env.sh \
                   "./scripts/tests/run_test_suite.py \
-                     --run-yamltests-with-chip-repl \
+                     --runner chip_repl_python \
                      run \
                      --iterations 1 \
                      --test-timeout-seconds 120 \
@@ -283,7 +302,7 @@ jobs:
 
     test_suites_darwin:
         name: Test Suites - Darwin
-        timeout-minutes: 150
+        timeout-minutes: 180
 
         strategy:
             matrix:
@@ -372,6 +391,26 @@ jobs:
                      --tv-app ./out/darwin-x64-tv-app-${BUILD_VARIANT}/chip-tv-app \
                      --bridge-app ./out/darwin-x64-bridge-${BUILD_VARIANT}/chip-bridge-app \
                   "
+
+            - name: Run Tests using the python parser sending commands to chip-tool
+              timeout-minutes: 80
+              run: |
+                  ./scripts/run_in_build_env.sh \
+                  "./scripts/tests/run_test_suite.py \
+                     --runner chip_tool_python \
+                     --chip-tool ./out/darwin-x64-chip-tool${CHIP_TOOL_VARIANT}-${BUILD_VARIANT}/chip-tool \
+                     --target-skip-glob '{Test_TC_DGTHREAD_2_1,Test_TC_DGTHREAD_2_2,Test_TC_DGTHREAD_2_3,Test_TC_DGTHREAD_2_4}' \
+                     run \
+                     --iterations 1 \
+                     --test-timeout-seconds 120 \
+                     --all-clusters-app ./out/darwin-x64-all-clusters-${BUILD_VARIANT}/chip-all-clusters-app \
+                     --lock-app ./out/darwin-x64-lock-${BUILD_VARIANT}/chip-lock-app \
+                     --ota-provider-app ./out/darwin-x64-ota-provider-${BUILD_VARIANT}/chip-ota-provider-app \
+                     --ota-requestor-app ./out/darwin-x64-ota-requestor-${BUILD_VARIANT}/chip-ota-requestor-app \
+                     --tv-app ./out/darwin-x64-tv-app-${BUILD_VARIANT}/chip-tv-app \
+                     --bridge-app ./out/darwin-x64-bridge-${BUILD_VARIANT}/chip-bridge-app \
+                  "
+
             - name: Uploading core files
               uses: actions/upload-artifact@v3
               if: ${{ failure() && !env.ACT }}

examples/chip-tool/commands/common/CHIPCommand.h

@@ -94,6 +94,14 @@ class CHIPCommand : public Command
     void SetCommandExitStatus(CHIP_ERROR status)
     {
         mCommandExitStatus = status;
+        // In interactive mode the stack is not shut down once a command is ended.
+        // That means calling `ErrorStr(err)` from the main thread when command
+        // completion is signaled may race since `ErrorStr` uses a static sErrorStr
+        // buffer for computing the error string.  Call it here instead.
+        if (IsInteractive() && CHIP_NO_ERROR != status)
+        {
+            ChipLogError(chipTool, "Run command failure: %s", chip::ErrorStr(status));
+        }
         StopWaiting();
     }
 

examples/chip-tool/commands/common/Commands.cpp

@@ -181,9 +181,7 @@ int Commands::RunInteractive(const char * command)
         delete[] argv[i];
     }
 
-    VerifyOrReturnValue(CHIP_NO_ERROR == err, EXIT_FAILURE, ChipLogError(chipTool, "Run command failure: %s", chip::ErrorStr(err)));
-
-    return EXIT_SUCCESS;
+    return (err == CHIP_NO_ERROR) ? EXIT_SUCCESS : EXIT_FAILURE;
 }
 
 CHIP_ERROR Commands::RunCommand(int argc, char ** argv, bool interactive)

examples/chip-tool/commands/discover/DiscoverCommissionablesCommand.cpp

@@ -29,49 +29,56 @@ void DiscoverCommissionablesCommandBase::OnDiscoveredDevice(const chip::Dnssd::D
 
     if (mDiscoverOnce.ValueOr(true))
     {
-        CurrentCommissioner().StopCommissionableDiscovery();
+        mCommissioner->RegisterDeviceDiscoveryDelegate(nullptr);
+        mCommissioner->StopCommissionableDiscovery();
         SetCommandExitStatus(CHIP_NO_ERROR);
     }
 }
 
 CHIP_ERROR DiscoverCommissionablesCommand::RunCommand()
 {
-    CurrentCommissioner().RegisterDeviceDiscoveryDelegate(this);
+    mCommissioner = &CurrentCommissioner();
+    mCommissioner->RegisterDeviceDiscoveryDelegate(this);
     Dnssd::DiscoveryFilter filter(Dnssd::DiscoveryFilterType::kNone, (uint64_t) 0);
-    return CurrentCommissioner().DiscoverCommissionableNodes(filter);
+    return mCommissioner->DiscoverCommissionableNodes(filter);
 }
 
 CHIP_ERROR DiscoverCommissionableByShortDiscriminatorCommand::RunCommand()
 {
-    CurrentCommissioner().RegisterDeviceDiscoveryDelegate(this);
+    mCommissioner = &CurrentCommissioner();
+    mCommissioner->RegisterDeviceDiscoveryDelegate(this);
     chip::Dnssd::DiscoveryFilter filter(chip::Dnssd::DiscoveryFilterType::kShortDiscriminator, mDiscriminator);
-    return CurrentCommissioner().DiscoverCommissionableNodes(filter);
+    return mCommissioner->DiscoverCommissionableNodes(filter);
 }
 
 CHIP_ERROR DiscoverCommissionableByLongDiscriminatorCommand::RunCommand()
 {
-    CurrentCommissioner().RegisterDeviceDiscoveryDelegate(this);
+    mCommissioner = &CurrentCommissioner();
+    mCommissioner->RegisterDeviceDiscoveryDelegate(this);
     chip::Dnssd::DiscoveryFilter filter(chip::Dnssd::DiscoveryFilterType::kLongDiscriminator, mDiscriminator);
-    return CurrentCommissioner().DiscoverCommissionableNodes(filter);
+    return mCommissioner->DiscoverCommissionableNodes(filter);
 }
 
 CHIP_ERROR DiscoverCommissionableByCommissioningModeCommand::RunCommand()
 {
-    CurrentCommissioner().RegisterDeviceDiscoveryDelegate(this);
+    mCommissioner = &CurrentCommissioner();
+    mCommissioner->RegisterDeviceDiscoveryDelegate(this);
     chip::Dnssd::DiscoveryFilter filter(chip::Dnssd::DiscoveryFilterType::kCommissioningMode);
-    return CurrentCommissioner().DiscoverCommissionableNodes(filter);
+    return mCommissioner->DiscoverCommissionableNodes(filter);
 }
 
 CHIP_ERROR DiscoverCommissionableByVendorIdCommand::RunCommand()
 {
-    CurrentCommissioner().RegisterDeviceDiscoveryDelegate(this);
+    mCommissioner = &CurrentCommissioner();
+    mCommissioner->RegisterDeviceDiscoveryDelegate(this);
     chip::Dnssd::DiscoveryFilter filter(chip::Dnssd::DiscoveryFilterType::kVendorId, mVendorId);
-    return CurrentCommissioner().DiscoverCommissionableNodes(filter);
+    return mCommissioner->DiscoverCommissionableNodes(filter);
 }
 
 CHIP_ERROR DiscoverCommissionableByDeviceTypeCommand::RunCommand()
 {
-    CurrentCommissioner().RegisterDeviceDiscoveryDelegate(this);
+    mCommissioner = &CurrentCommissioner();
+    mCommissioner->RegisterDeviceDiscoveryDelegate(this);
     chip::Dnssd::DiscoveryFilter filter(chip::Dnssd::DiscoveryFilterType::kDeviceType, mDeviceType);
-    return CurrentCommissioner().DiscoverCommissionableNodes(filter);
+    return mCommissioner->DiscoverCommissionableNodes(filter);
 }

examples/chip-tool/commands/discover/DiscoverCommissionablesCommand.h

@@ -36,6 +36,9 @@ class DiscoverCommissionablesCommandBase : public CHIPCommand, public chip::Cont
     /////////// CHIPCommand Interface /////////
     chip::System::Clock::Timeout GetWaitDuration() const override { return chip::System::Clock::Seconds16(30); }
 
+protected:
+    chip::Controller::DeviceCommissioner * mCommissioner;
+
 private:
     chip::Optional<bool> mDiscoverOnce;
 };

examples/chip-tool/commands/interactive/InteractiveCommands.cpp

@@ -43,6 +43,17 @@ void ENFORCE_FORMAT(3, 0) LoggingCallback(const char * module, uint8_t category,
     ClearLine();
 }
 
+class ScopedLock
+{
+public:
+    ScopedLock(std::mutex & mutex) : mMutex(mutex) { mMutex.lock(); }
+
+    ~ScopedLock() { mMutex.unlock(); }
+
+private:
+    std::mutex & mMutex;
+};
+
 struct InteractiveServerResultLog
 {
     std::string module;
@@ -58,25 +69,55 @@ struct InteractiveServerResult
     std::vector<std::string> mResults;
     std::vector<InteractiveServerResultLog> mLogs;
 
+    // The InteractiveServerResult instance (gInteractiveServerResult) is initially
+    // accessed on the main thread in InteractiveServerCommand::RunCommand, which is
+    // when chip-tool starts in 'interactive server' mode.
+    //
+    // Then command results are normally sent over the wire onto the main thread too
+    // when a command is received over WebSocket in InteractiveServerCommand::OnWebSocketMessageReceived
+    // which for most cases runs a command onto the chip thread and block until
+    // it is resolved (or until it timeouts).
+    //
+    // But in the meantime, when some parts of the command result happens, it is appended
+    // to the mResults vector onto the chip thread.
+    //
+    // For empty commands, which means that the test suite is *waiting* for some events
+    // (e.g a subscription report), the command results are sent over the chip thread
+    // (this is the isAsyncReport use case).
+    //
+    // Finally, logs can be appended from either the chip thread or the main thread.
+    //
+    // This class should be refactored to abstract that properly and reduce the scope of
+    // of the mutex, but in the meantime, the access to the members of this class are
+    // protected by a mutex.
+    std::mutex mMutex;
+
     void Setup(bool isAsyncReport)
     {
+        auto lock      = ScopedLock(mMutex);
         mEnabled       = true;
         mIsAsyncReport = isAsyncReport;
     }
 
     void Reset()
     {
+        auto lock      = ScopedLock(mMutex);
         mEnabled       = false;
         mIsAsyncReport = false;
         mStatus        = EXIT_SUCCESS;
         mResults.clear();
         mLogs.clear();
     }
 
-    bool IsAsyncReport() const { return mIsAsyncReport; }
+    bool IsAsyncReport()
+    {
+        auto lock = ScopedLock(mMutex);
+        return mIsAsyncReport;
+    }
 
     void MaybeAddLog(const char * module, uint8_t category, const char * base64Message)
     {
+        auto lock = ScopedLock(mMutex);
         VerifyOrReturn(mEnabled);
 
         const char * messageType = nullptr;
@@ -98,12 +139,16 @@ struct InteractiveServerResult
 
     void MaybeAddResult(const char * result)
     {
+        auto lock = ScopedLock(mMutex);
         VerifyOrReturn(mEnabled);
+
         mResults.push_back(result);
     }
 
-    std::string AsJsonString() const
+    std::string AsJsonString()
     {
+        auto lock = ScopedLock(mMutex);
+
         std::string resultsStr;
         if (mResults.size())
         {
@@ -205,21 +250,15 @@ CHIP_ERROR InteractiveServerCommand::RunCommand()
     return CHIP_NO_ERROR;
 }
 
-void SendOverWebSocket(intptr_t context)
-{
-    auto server = reinterpret_cast<WebSocketServer *>(context);
-    server->Send(gInteractiveServerResult.AsJsonString().c_str());
-    gInteractiveServerResult.Reset();
-}
-
 bool InteractiveServerCommand::OnWebSocketMessageReceived(char * msg)
 {
     bool isAsyncReport = strlen(msg) == 0;
     gInteractiveServerResult.Setup(isAsyncReport);
     VerifyOrReturnValue(!isAsyncReport, true);
 
     auto shouldStop = ParseCommand(msg, &gInteractiveServerResult.mStatus);
-    chip::DeviceLayer::PlatformMgr().ScheduleWork(SendOverWebSocket, reinterpret_cast<intptr_t>(&mWebSocketServer));
+    mWebSocketServer.Send(gInteractiveServerResult.AsJsonString().c_str());
+    gInteractiveServerResult.Reset();
     return shouldStop;
 }
 
@@ -228,7 +267,8 @@ CHIP_ERROR InteractiveServerCommand::LogJSON(const char * json)
     gInteractiveServerResult.MaybeAddResult(json);
     if (gInteractiveServerResult.IsAsyncReport())
     {
-        chip::DeviceLayer::PlatformMgr().ScheduleWork(SendOverWebSocket, reinterpret_cast<intptr_t>(&mWebSocketServer));
+        mWebSocketServer.Send(gInteractiveServerResult.AsJsonString().c_str());
+        gInteractiveServerResult.Reset();
     }
     return CHIP_NO_ERROR;
 }

examples/chip-tool/py_matter_chip_tool_adapter/BUILD.gn

@@ -0,0 +1,38 @@
+# Copyright (c) 2023 Project CHIP Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import("//build_overrides/build.gni")
+import("//build_overrides/chip.gni")
+
+import("//build_overrides/pigweed.gni")
+import("$dir_pw_build/python.gni")
+
+pw_python_package("matter_chip_tool_adapter") {
+  setup = [
+    "setup.py",
+    "setup.cfg",
+    "pyproject.toml",
+  ]
+
+  sources = [
+    "matter_chip_tool_adapter/__init__.py",
+    "matter_chip_tool_adapter/adapter.py",
+    "matter_chip_tool_adapter/decoder.py",
+    "matter_chip_tool_adapter/encoder.py",
+  ]
+
+  # TODO: at a future time consider enabling all (* or missing) here to get
+  #       pylint checking these files
+  static_analysis = []
+}

examples/chip-tool/py_matter_chip_tool_adapter/matter_chip_tool_adapter/adapter.py

@@ -0,0 +1,28 @@
+# Copyright (c) 2023 Project CHIP Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from .decoder import Decoder
+from .encoder import Encoder
+
+
+class Adapter:
+    def __init__(self, specifications):
+        self.encoder = Encoder(specifications)
+        self.decoder = Decoder(specifications)
+
+    def encode(self, request):
+        return self.encoder.encode(request)
+
+    def decode(self, response):
+        return self.decoder.decode(response)