Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

TestReadInteraction exercises ReadHandler in ways that can never happen in practice #8031

Closed
bzbarsky-apple opened this issue Jun 30, 2021 · 0 comments · Fixed by #15370
Closed

TestReadInteraction exercises ReadHandler in ways that can never happen in practice #8031

bzbarsky-apple opened this issue Jun 30, 2021 · 0 comments · Fixed by #15370
Assignees
@yunhanw-google
Labels
Interaction Model Work V1.X

Comments

@bzbarsky-apple
Copy link
Contributor

bzbarsky-apple commented Jun 30, 2021 •
edited
Loading

Problem

TestReadInteraction does readHandler.OnReadRequest(nullptr, std::move(readRequestbuf)). That nullptr is the exchange pointer. That's not something that can ever happen in actual operation, and the code in ReadHandler has to do some hoop-jumping to handle this test-only case.

Proposed Solution

Exercise this more reasonably. Probably by actually dispatching the read request buffer via the exchange manager, so it creates an exchange, does the dispatch via the unsolicited message handler, etc.

When this is fixed, the workaround I am adding in ReadHandler::ProcessReadRequest for null exchange context should be removed.
@yunhanw-google yunhanw-google self-assigned this Aug 27, 2021
bzbarsky-apple added a commit to bzbarsky-apple/connectedhomeip that referenced this issue Feb 19, 2022
@bzbarsky-apple
Remove unnecessary null-checks for exchange contexts.
b028193 
The unit tests for this code no longer call it without an exchange
context, so we can remove the workarounds.

Fixes project-chip#8031
@bzbarsky-apple bzbarsky-apple mentioned this issue Feb 19, 2022
Merged
woody-apple pushed a commit to bzbarsky-apple/connectedhomeip that referenced this issue Feb 19, 2022
@bzbarsky-apple
Remove unnecessary null-checks for exchange contexts.
a168bff 
The unit tests for this code no longer call it without an exchange
context, so we can remove the workarounds.

Fixes project-chip#8031
bzbarsky-apple added a commit that referenced this issue Feb 21, 2022
@bzbarsky-apple
Remove unnecessary null-checks for exchange contexts. (#15370)
498980d 
The unit tests for this code no longer call it without an exchange
context, so we can remove the workarounds.

Fixes #8031
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@yunhanw-google yunhanw-google

Labels
Interaction Model Work V1.X
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove unnecessary null-checks for exchange contexts. bzbarsky-apple/connectedhomeip
3 participants
@yunhanw-google @woody-apple @bzbarsky-apple