From a4a3370c6af9c95264623ca5cb9f1a70abd32760 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Wed, 1 Mar 2023 13:47:46 -0500 Subject: [PATCH] Remove flaky test from list now that delay is added (#25256) * Remove flaky test from list now that delay is added * Fix CI * Fix CI * Address PR comments * Address PR comments * Fix formatting * Change to delay as per Chaitanya's comments * Restyle --- scripts/tests/chiptest/__init__.py | 10 ++-- .../suites/certification/Test_TC_OO_2_4.yaml | 19 ++++++++ .../chip-tool/zap-generated/test/Commands.h | 28 +++++++---- .../zap-generated/test/Commands.h | 47 +++++++++++++------ 4 files changed, 78 insertions(+), 26 deletions(-) diff --git a/scripts/tests/chiptest/__init__.py b/scripts/tests/chiptest/__init__.py index 55e125c29bac19..7692ac08e8074f 100644 --- a/scripts/tests/chiptest/__init__.py +++ b/scripts/tests/chiptest/__init__.py @@ -84,10 +84,12 @@ def _GetManualTests() -> Set[str]: def _GetFlakyTests() -> Set[str]: - """List of flaky tests, ideally this list should become empty.""" - return { - "Test_TC_OO_2_4.yaml" - } + """List of flaky tests. + + While this list is empty, it remains here in case we need to quickly add a new test + that is flaky. + """ + return set() def _GetSlowTests() -> Set[str]: diff --git a/src/app/tests/suites/certification/Test_TC_OO_2_4.yaml b/src/app/tests/suites/certification/Test_TC_OO_2_4.yaml index 22538ffbf08565..736a8706b92072 100644 --- a/src/app/tests/suites/certification/Test_TC_OO_2_4.yaml +++ b/src/app/tests/suites/certification/Test_TC_OO_2_4.yaml @@ -238,6 +238,25 @@ tests: PICS: OO.S.C00.Rsp command: "Off" + # We get an acknowledgment that the command off above is received, but it takes + # a few milliseconds for the off command to actually turn it off. Without this + # delay the preceding reboot command might happen before the command is fully + # processed resulting in flaky test failure that comes from the readAddribute + # verification after the reboot. + # TODO once https://github.com/CHIP-Specifications/chip-test-plans/pull/2464 is + # merged, change this command to a readAtribute and update PICS code to + # OO.S.A0000. PR is blocked from being merged at this time due to freeze for + # SVE. Likely after March 17th, 2023 this PR can be merged and associated + # changes made here to officially fix the flake discovered by CI. + - label: "Wait for send Off command to take affect" + PICS: PICS_SDK_CI_ONLY + cluster: "DelayCommands" + command: "WaitForMs" + arguments: + values: + - name: "ms" + value: 10 + - label: "Reboot target device" PICS: PICS_SDK_CI_ONLY cluster: "SystemCommands" diff --git a/zzz_generated/chip-tool/zap-generated/test/Commands.h b/zzz_generated/chip-tool/zap-generated/test/Commands.h index e090a600cff0d5..a4af2886981692 100644 --- a/zzz_generated/chip-tool/zap-generated/test/Commands.h +++ b/zzz_generated/chip-tool/zap-generated/test/Commands.h @@ -37425,7 +37425,7 @@ class Test_TC_OO_2_2Suite : public TestCommand class Test_TC_OO_2_4Suite : public TestCommand { public: - Test_TC_OO_2_4Suite(CredentialIssuerCommands * credsIssuerConfig) : TestCommand("Test_TC_OO_2_4", 31, credsIssuerConfig) + Test_TC_OO_2_4Suite(CredentialIssuerCommands * credsIssuerConfig) : TestCommand("Test_TC_OO_2_4", 32, credsIssuerConfig) { AddArgument("nodeId", 0, UINT64_MAX, &mNodeId); AddArgument("cluster", &mCluster); @@ -37593,6 +37593,10 @@ class Test_TC_OO_2_4Suite : public TestCommand shouldContinue = true; break; case 30: + VerifyOrReturn(CheckValue("status", chip::to_underlying(status.mStatus), 0)); + shouldContinue = true; + break; + case 31: VerifyOrReturn(CheckValue("status", chip::to_underlying(status.mStatus), 0)); { bool value; @@ -37830,14 +37834,22 @@ class Test_TC_OO_2_4Suite : public TestCommand ); } case 27: { - LogStep(27, "Reboot target device"); + LogStep(27, "Wait for send Off command to take affect"); + VerifyOrDo(!ShouldSkip("PICS_SDK_CI_ONLY"), return ContinueOnChipMainThread(CHIP_NO_ERROR)); + ListFreer listFreer; + chip::app::Clusters::DelayCommands::Commands::WaitForMs::Type value; + value.ms = 10UL; + return WaitForMs(kIdentityAlpha, value); + } + case 28: { + LogStep(28, "Reboot target device"); VerifyOrDo(!ShouldSkip("PICS_SDK_CI_ONLY"), return ContinueOnChipMainThread(CHIP_NO_ERROR)); ListFreer listFreer; chip::app::Clusters::SystemCommands::Commands::Reboot::Type value; return Reboot(kIdentityAlpha, value); } - case 28: { - LogStep(28, "Reboot target device(DUT)"); + case 29: { + LogStep(29, "Reboot target device(DUT)"); VerifyOrDo(!ShouldSkip("PICS_SKIP_SAMPLE_APP"), return ContinueOnChipMainThread(CHIP_NO_ERROR)); ListFreer listFreer; chip::app::Clusters::LogCommands::Commands::UserPrompt::Type value; @@ -37847,15 +37859,15 @@ class Test_TC_OO_2_4Suite : public TestCommand value.expectedValue.Value() = chip::Span("ygarbage: not in length on purpose", 1); return UserPrompt(kIdentityAlpha, value); } - case 29: { - LogStep(29, "Wait for the commissioned device to be retrieved"); + case 30: { + LogStep(30, "Wait for the commissioned device to be retrieved"); ListFreer listFreer; chip::app::Clusters::DelayCommands::Commands::WaitForCommissionee::Type value; value.nodeId = mNodeId.HasValue() ? mNodeId.Value() : 305414945ULL; return WaitForCommissionee(kIdentityAlpha, value); } - case 30: { - LogStep(30, "TH reads the OnOff attribute from the DUT"); + case 31: { + LogStep(31, "TH reads the OnOff attribute from the DUT"); VerifyOrDo(!ShouldSkip("OO.S.A0000"), return ContinueOnChipMainThread(CHIP_NO_ERROR)); return ReadAttribute(kIdentityAlpha, GetEndpoint(1), OnOff::Id, OnOff::Attributes::OnOff::Id, true, chip::NullOptional); } diff --git a/zzz_generated/darwin-framework-tool/zap-generated/test/Commands.h b/zzz_generated/darwin-framework-tool/zap-generated/test/Commands.h index 14f02c16167c3e..5e152698339007 100644 --- a/zzz_generated/darwin-framework-tool/zap-generated/test/Commands.h +++ b/zzz_generated/darwin-framework-tool/zap-generated/test/Commands.h @@ -50730,32 +50730,40 @@ class Test_TC_OO_2_4 : public TestCommandBridge { err = TestThSendsOffCommandToDut_26(); break; case 27: - ChipLogProgress(chipTool, " ***** Test Step 27 : Reboot target device\n"); + ChipLogProgress(chipTool, " ***** Test Step 27 : Wait for send Off command to take affect\n"); if (ShouldSkip("PICS_SDK_CI_ONLY")) { NextTest(); return; } - err = TestRebootTargetDevice_27(); + err = TestWaitForSendOffCommandToTakeAffect_27(); break; case 28: - ChipLogProgress(chipTool, " ***** Test Step 28 : Reboot target device(DUT)\n"); - if (ShouldSkip("PICS_SKIP_SAMPLE_APP")) { + ChipLogProgress(chipTool, " ***** Test Step 28 : Reboot target device\n"); + if (ShouldSkip("PICS_SDK_CI_ONLY")) { NextTest(); return; } - err = TestRebootTargetDeviceDUT_28(); + err = TestRebootTargetDevice_28(); break; case 29: - ChipLogProgress(chipTool, " ***** Test Step 29 : Wait for the commissioned device to be retrieved\n"); - err = TestWaitForTheCommissionedDeviceToBeRetrieved_29(); + ChipLogProgress(chipTool, " ***** Test Step 29 : Reboot target device(DUT)\n"); + if (ShouldSkip("PICS_SKIP_SAMPLE_APP")) { + NextTest(); + return; + } + err = TestRebootTargetDeviceDUT_29(); break; case 30: - ChipLogProgress(chipTool, " ***** Test Step 30 : TH reads the OnOff attribute from the DUT\n"); + ChipLogProgress(chipTool, " ***** Test Step 30 : Wait for the commissioned device to be retrieved\n"); + err = TestWaitForTheCommissionedDeviceToBeRetrieved_30(); + break; + case 31: + ChipLogProgress(chipTool, " ***** Test Step 31 : TH reads the OnOff attribute from the DUT\n"); if (ShouldSkip("OO.S.A0000")) { NextTest(); return; } - err = TestThReadsTheOnOffAttributeFromTheDut_30(); + err = TestThReadsTheOnOffAttributeFromTheDut_31(); break; } @@ -50861,6 +50869,9 @@ class Test_TC_OO_2_4 : public TestCommandBridge { case 30: VerifyOrReturn(CheckValue("status", chip::to_underlying(status.mStatus), 0)); break; + case 31: + VerifyOrReturn(CheckValue("status", chip::to_underlying(status.mStatus), 0)); + break; } // Go on to the next test. @@ -50874,7 +50885,7 @@ class Test_TC_OO_2_4 : public TestCommandBridge { private: std::atomic_uint16_t mTestIndex; - const uint16_t mTestCount = 31; + const uint16_t mTestCount = 32; chip::Optional mNodeId; chip::Optional mCluster; @@ -51254,14 +51265,22 @@ class Test_TC_OO_2_4 : public TestCommandBridge { return CHIP_NO_ERROR; } - CHIP_ERROR TestRebootTargetDevice_27() + CHIP_ERROR TestWaitForSendOffCommandToTakeAffect_27() + { + + chip::app::Clusters::DelayCommands::Commands::WaitForMs::Type value; + value.ms = 10UL; + return WaitForMs("alpha", value); + } + + CHIP_ERROR TestRebootTargetDevice_28() { chip::app::Clusters::SystemCommands::Commands::Reboot::Type value; return Reboot("alpha", value); } - CHIP_ERROR TestRebootTargetDeviceDUT_28() + CHIP_ERROR TestRebootTargetDeviceDUT_29() { chip::app::Clusters::LogCommands::Commands::UserPrompt::Type value; @@ -51272,7 +51291,7 @@ class Test_TC_OO_2_4 : public TestCommandBridge { return UserPrompt("alpha", value); } - CHIP_ERROR TestWaitForTheCommissionedDeviceToBeRetrieved_29() + CHIP_ERROR TestWaitForTheCommissionedDeviceToBeRetrieved_30() { chip::app::Clusters::DelayCommands::Commands::WaitForCommissionee::Type value; @@ -51280,7 +51299,7 @@ class Test_TC_OO_2_4 : public TestCommandBridge { return WaitForCommissionee("alpha", value); } - CHIP_ERROR TestThReadsTheOnOffAttributeFromTheDut_30() + CHIP_ERROR TestThReadsTheOnOffAttributeFromTheDut_31() { MTRBaseDevice * device = GetDevice("alpha");