project-chip / connectedhomeip

Matter (formerly Project CHIP) creates more connections between more objects, simplifying development for manufacturers and increasing compatibility for consumers, guided by the Connectivity Standards Alliance.
https://buildwithmatter.com
Apache License 2.0
7.55k stars 2.04k forks source link

CI cirque tests are fragile, tightly coupled to commissioning steps #21313

Open chrisdecenzo opened 2 years ago

chrisdecenzo commented 2 years ago

Problem

When adding new optional steps to src/controller/CommissioningDelegate.h, the circ tests began failing

There is a set of "failure" tests, one for each step, which force a failure at the given step. It's a negative test, so it expects the setup to fail when an individual step fails. Since the controller skips over the scanNetworks step, the test case which forces a failure at that step never trips, and so the setup succeeds rather than fails. I discovered that when I put this step at the end of the list, we don't have a problem since the "failure" test set seems to only cover a limited number of steps.

These tests need to be fixed so that they are not fragile when additional steps are added to the commissioning flow. The result is that additional steps need to be added to the end of the list, out of order to when they actually would fire

Reference: https://github.com/project-chip/connectedhomeip/pull/20766

Proposed Solution

Find a way to de-couple these failure test cases, for example, by marking the test as invalid if the step in question (sent via a breadcrumb) does not actually fire.

bzbarsky-apple commented 2 years ago

@mrjerryjohns @cecille

mrjerryjohns commented 2 years ago

@cecille could you help take a look at this?

cecille commented 2 years ago

The class used to simulate failures is TestCommissioner (src/controller/python/OpCredsBinding.cpp). There's function "ValidStage" that tests whether it's OK to simulate a failure on that stage. If you add a check in there (return false for new stage), we won't attempt to simulate failures on that stage.

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.