From efa7aaa565c86f55a07fde131146c9633a2e22be Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Tue, 8 Jun 2021 14:05:31 -0400 Subject: [PATCH] Fix incorrect test termination condition in TestCommand subclasses. (#7428) The logic in NextTest was doing the following: 1. Start the test at mTestIndex. 2. Increment mTestIndex. 3. If mTestCount == mTestIndex (i.e. no more tests to run), start shutdown (by calling SetCommandExitStatus, which unblocks the shutdown sequence).. This failed when running the last test: we would start the test, increment mTestIndex, then hit that mTestCount condition and immediately start shutdown, before waiting for the last test to complete. The fix is to test for the "no more tests to run" condition on entry to NextTest. This way the last test runs compeletely, and when it's done and calls NextTest we go ahead and shut down. --- examples/chip-tool/commands/tests/Commands.h | 20 +++++++++++++++---- .../templates/partials/test_cluster.zapt | 10 ++++++++-- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/examples/chip-tool/commands/tests/Commands.h b/examples/chip-tool/commands/tests/Commands.h index 37d9ffdfd8a335..807a9bbe4e4f37 100644 --- a/examples/chip-tool/commands/tests/Commands.h +++ b/examples/chip-tool/commands/tests/Commands.h @@ -31,6 +31,12 @@ class TestCluster : public TestCommand { CHIP_ERROR err = CHIP_NO_ERROR; + if (mTestCount == mTestIndex) + { + ChipLogProgress(chipTool, "TestCluster: Test complete"); + SetCommandExitStatus(true); + } + switch (mTestIndex) { case 0: @@ -51,10 +57,10 @@ class TestCluster : public TestCommand } mTestIndex++; - if (mTestCount == mTestIndex || CHIP_NO_ERROR != err) + if (CHIP_NO_ERROR != err) { ChipLogProgress(chipTool, "TestCluster: %s", chip::ErrorStr(err)); - SetCommandExitStatus(CHIP_NO_ERROR == err); + SetCommandExitStatus(false); } return err; @@ -438,6 +444,12 @@ class OnOffCluster : public TestCommand { CHIP_ERROR err = CHIP_NO_ERROR; + if (mTestCount == mTestIndex) + { + ChipLogProgress(chipTool, "OnOffCluster: Test complete"); + SetCommandExitStatus(true); + } + switch (mTestIndex) { case 0: @@ -458,10 +470,10 @@ class OnOffCluster : public TestCommand } mTestIndex++; - if (mTestCount == mTestIndex || CHIP_NO_ERROR != err) + if (CHIP_NO_ERROR != err) { ChipLogProgress(chipTool, "OnOffCluster: %s", chip::ErrorStr(err)); - SetCommandExitStatus(CHIP_NO_ERROR == err); + SetCommandExitStatus(false); } return err; diff --git a/examples/chip-tool/templates/partials/test_cluster.zapt b/examples/chip-tool/templates/partials/test_cluster.zapt index e8393319139364..2a201787bb4525 100644 --- a/examples/chip-tool/templates/partials/test_cluster.zapt +++ b/examples/chip-tool/templates/partials/test_cluster.zapt @@ -9,6 +9,12 @@ class {{asCamelCased filename false}}: public TestCommand { CHIP_ERROR err = CHIP_NO_ERROR; + if (mTestCount == mTestIndex) + { + ChipLogProgress(chipTool, "{{filename}}: Test complete"); + SetCommandExitStatus(true); + } + switch (mTestIndex) { {{#chip_tests_items}} @@ -19,10 +25,10 @@ class {{asCamelCased filename false}}: public TestCommand } mTestIndex++; - if (mTestCount == mTestIndex || CHIP_NO_ERROR != err) + if (CHIP_NO_ERROR != err) { ChipLogProgress(chipTool, "{{filename}}: %s", chip::ErrorStr(err)); - SetCommandExitStatus(CHIP_NO_ERROR == err); + SetCommandExitStatus(false); } return err;