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)