Add test for sending a command to an unsupported endpoint #8073
Conversation
pullapprove
bot
requested review from
andy31415,
chrisdecenzo,
Damian-Nordic,
hawk248,
jepenven-silabs,
kedars,
msandstedt and
woody-apple
| runner->NextTest(); | ||
| } | ||
|
|
||
| static void OnTestSendClusterTestClusterCommandTest_101_SuccessResponse(void * context) |
There was a problem hiding this comment.
Looks like this pattern repeats for other test cases as well. Could this callback functions be common for all of them? Is there a reason we can't have a common runner->mIsFailureExpected member?
There was a problem hiding this comment.
Possibly. This is auto-generated code; I'd have to go look at the template to see whether we could do that and whether the resulting template would be more complicated or not as a result....
There was a problem hiding this comment.
The template has various conditions in it; depending on the exact way the test is configured in the yaml you get different signatures. So commoning up the functions is not really very simple.
Similarly, mIsFailureExpected is on a per-test basis, and the runner is a single runner across a suite of tests, so we can't just have one member on it to track the per-test state.
|
|
||
| // Test Send Test Command to unsupported endpoint | ||
| typedef void (*SuccessCallback_101)(void * context); | ||
| chip::Callback::Callback<SuccessCallback_101> * mOnSuccessCallback_101 = nullptr; |
There was a problem hiding this comment.
Why does this not say:
chip::Callback::Callback<SuccessCallback_101> mOnSuccessCallback_101 { &OnTestSendClusterTestClusterCommandTest_101_FailureResponse, this };
And if it needs to be nullable for some reason (I'm not sure why..) we could use Optional<> or std::unique_ptr.
I think this is worse than all of 3 other possibilities.
There was a problem hiding this comment.
Why does this not say:
I'm happy to look into this, but this is auto-generated code (see the two separate commits which break out the manual changes and the generated changes) so the fix would be pretty orthogonal and would affect all of the test commands, presumably.
There was a problem hiding this comment.
So there is no reason this has to be nullable. That said, right now we allocate the callback for a test right before we start that test, and deallocate it when the test finishes. If we just made them non-nullable members we'd have them all allocated all the time, effectively. That's probably not a problem for the sorts of devices that run chip-tool....
| @@ -7829,6 +7832,75 @@ class TestCluster : public TestCommand | |||
|
|
|||
| runner->NextTest(); | |||
| } | |||
|
|
|||
| // Test Send Test Command to unsupported endpoint | |||
| typedef void (*SuccessCallback_101)(void * context); | |||
There was a problem hiding this comment.
nit: Prefer usingas it is more readable than typedef
There was a problem hiding this comment.
Again, this is generated code. Happy to look into this in a followup, but don't think we should be including changes to the generator templates in this PR.
…ist. The test script change is to allow it to detect crashes in the server app as failures (due to the server app not being there to be killed).
bzbarsky-apple
force-pushed
the
add-test-for-unknown-endpoint
branch
from
4f4a5bf to
32b4002
Compare
…ip#8073) * Add test for trying to send a command to an endpoint that does not exist. The test script change is to allow it to detect crashes in the server app as failures (due to the server app not being there to be killed). * Regenerate generated files
Problem
We have no test coverage for sending a command to an unsupported endpoint.
Change overview
Add a test.
Testing
Ran Darwin tests and
test_suites.shmanually, verified the test fails. And will keep failing until #8054 merges.