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.

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

Add configuration option for more verbose VerifyOrDie messages. #9800

Merged
merged 1 commit into from
Sep 17, 2021

Conversation

bzbarsky-apple
Copy link
Contributor

We have a lot of VerifyOrDie with no messages, and when they fail it's
hard to tell what went wrong. This change adds a configuration option
to automatically generate a verbose message string for VerifyOrDie if
one is not provided. The option is off by default but enabled for
Darwin and Linux.

Log from a failing unit test without this change:

[1631900831920] [73638:57013508] CHIP: [EM] Piggybacking Ack for MessageCounter:0000000D with msg
../../third_party/pigweed/repo/targets/host/run_test: line 18: 73638 Abort trap: 6           "$@"
INF Test 1/1: [FAIL] TestReliableMessageProtocol

Log from the same failure with this change:

[1631900926163] [79197:57035322] CHIP: [EM] Piggybacking Ack for MessageCounter:0000000D with msg
[1631900926163] [79197:57035322] CHIP: [SPT] VerifyOrDie failure at ../../src/messaging/ReliableMessageMgr.cpp:262: rc != nullptr && !rc->IsOccupied()
../../third_party/pigweed/repo/targets/host/run_test: line 18: 79197 Abort trap: 6           "$@"
INF Test 1/1: [FAIL] TestReliableMessageProtocol

Problem

See above

Change overview

See above

Testing

Manually ran a unit test that fails in VerifyOrDie on Mac; see logs above.

We have a lot of VerifyOrDie with no messages, and when they fail it's
hard to tell what went wrong.  This change adds a configuration option
to automatically generate a verbose message string for VerifyOrDie if
one is not provided.  The option is off by default but enabled for
Darwin and Linux.

Log from a failing unit test without this change:

[1631900831920] [73638:57013508] CHIP: [EM] Piggybacking Ack for MessageCounter:0000000D with msg
../../third_party/pigweed/repo/targets/host/run_test: line 18: 73638 Abort trap: 6           "$@"
INF Test 1/1: [FAIL] TestReliableMessageProtocol

Log from the same failure with this change:

[1631900926163] [79197:57035322] CHIP: [EM] Piggybacking Ack for MessageCounter:0000000D with msg
[1631900926163] [79197:57035322] CHIP: [SPT] VerifyOrDie failure at ../../src/messaging/ReliableMessageMgr.cpp:262: rc != nullptr && !rc->IsOccupied()
../../third_party/pigweed/repo/targets/host/run_test: line 18: 79197 Abort trap: 6           "$@"
INF Test 1/1: [FAIL] TestReliableMessageProtocol
@andy31415 andy31415 merged commit 37a6d27 into project-chip:master Sep 17, 2021
@bzbarsky-apple bzbarsky-apple deleted the verbose-verify-or-die branch September 18, 2021 00:53
mleisner pushed a commit to mleisner/connectedhomeip that referenced this pull request Sep 20, 2021
…ect-chip#9800)

We have a lot of VerifyOrDie with no messages, and when they fail it's
hard to tell what went wrong.  This change adds a configuration option
to automatically generate a verbose message string for VerifyOrDie if
one is not provided.  The option is off by default but enabled for
Darwin and Linux.

Log from a failing unit test without this change:

[1631900831920] [73638:57013508] CHIP: [EM] Piggybacking Ack for MessageCounter:0000000D with msg
../../third_party/pigweed/repo/targets/host/run_test: line 18: 73638 Abort trap: 6           "$@"
INF Test 1/1: [FAIL] TestReliableMessageProtocol

Log from the same failure with this change:

[1631900926163] [79197:57035322] CHIP: [EM] Piggybacking Ack for MessageCounter:0000000D with msg
[1631900926163] [79197:57035322] CHIP: [SPT] VerifyOrDie failure at ../../src/messaging/ReliableMessageMgr.cpp:262: rc != nullptr && !rc->IsOccupied()
../../third_party/pigweed/repo/targets/host/run_test: line 18: 79197 Abort trap: 6           "$@"
INF Test 1/1: [FAIL] TestReliableMessageProtocol
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants