Closed Bug 933841 Opened 6 years ago Closed 6 years ago

Dump RLogRingBuffer on test failures

Categories

(Core :: WebRTC: Networking, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla28

People

(Reporter: bwc, Assigned: bwc)

References

Details

Attachments

(1 file, 4 obsolete files)

Once RLogRingBuffer is in place, the signalling unit tests in media/mtransport should dump the ringbuffer on test failure.
Initial patch. Probably has threading problems, but it does dump logs on failure.
Assignee: nobody → docfaraday
Status: NEW → ASSIGNED
Verified that the threading stuff was ok. Finishing up.
Attachment #826033 - Attachment is obsolete: true
Attachment #826052 - Flags: review?(ekr)
This test uses an actual STS thread. Dispatch the ringbuffer stuff to it.
Attachment #826052 - Attachment is obsolete: true
Attachment #826052 - Flags: review?(ekr)
Comment on attachment 826099 [details] [diff] [review]
Add event handler to dump RLogRingBuffer on test failure, and clear RLogRingBuffer on test start.

Review of attachment 826099 [details] [diff] [review]:
-----------------------------------------------------------------

This RingbufferDumper class could be broken off into its own file, and used elsewhere. Do we want to do that?
Attachment #826099 - Flags: review?(ekr)
Comment on attachment 826099 [details] [diff] [review]
Add event handler to dump RLogRingBuffer on test failure, and clear RLogRingBuffer on test start.

Review of attachment 826099 [details] [diff] [review]:
-----------------------------------------------------------------

::: media/webrtc/signaling/test/signaling_unittests.cpp
@@ +105,5 @@
>  namespace test {
> +class RingbufferDumper : public ::testing::EmptyTestEventListener {
> +  void ClearRingBuffer_s() {
> +    RLogRingBuffer::CreateInstance();
> +    RLogRingBuffer::GetInstance()->SetLogLimit(0);

Can you add some comments explaining what this does.

@@ +106,5 @@
> +class RingbufferDumper : public ::testing::EmptyTestEventListener {
> +  void ClearRingBuffer_s() {
> +    RLogRingBuffer::CreateInstance();
> +    RLogRingBuffer::GetInstance()->SetLogLimit(0);
> +    RLogRingBuffer::GetInstance()->SetLogLimit(4096);

Suggest making this unlimited? What's the worst that caould happen?

@@ +115,5 @@
> +  }
> +
> +  void DumpRingBuffer_s() {
> +      std::deque<std::string> logs;
> +      RLogRingBuffer::GetInstance()->GetAny(0, &logs);

Can we put some banner here to say what's going on?
Attachment #826099 - Flags: review?(ekr) → review+
Attachment #826099 - Attachment is obsolete: true
Going to make sure this applies cleanly.
Attachment #831775 - Attachment is obsolete: true
Comment on attachment 831815 [details] [diff] [review]
Add event handler to dump RLogRingBuffer on test failure, and clear RLogRingBuffer on test start.

Review of attachment 831815 [details] [diff] [review]:
-----------------------------------------------------------------

Carry forward r+ from ekr, requesting checkin.
Attachment #831815 - Flags: review+
Attachment #831815 - Flags: checkin?(adam)
Comment on attachment 831815 [details] [diff] [review]
Add event handler to dump RLogRingBuffer on test failure, and clear RLogRingBuffer on test start.

https://hg.mozilla.org/integration/mozilla-inbound/rev/4a68f748d240
Attachment #831815 - Flags: checkin?(adam) → checkin+
https://hg.mozilla.org/mozilla-central/rev/4a68f748d240
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
You need to log in before you can comment on or make changes to this bug.