[asterisk-dev] [Code Review]: Get mixmonitor_audiohook_inherit test working again

[jrose]

> On Oct. 1, 2012, 3:38 p.m., Mark Michelson wrote:
> > So, a problem I have with this is that as far as I can tell, the mechanics of this test are still exactly the same as they were previously. The previous test had a 60 second timeout built in that's not here, but the TestCase has its own timeout built in instead, so that's really not a change. My fear is that this version of the test will bounce just as the previous version did.
> > 
> > The logs from test failures are likely long gone and the 'skip' line in the test-config.yaml file gave no indication to you why the test was failing, so it's hard to guess what might have been the problem. However, there are a couple of things about this test that throw up red flags for me.
> > 
> > 1) The blind transfer in the dialplan. This seems like something that could easily be "missed" by Asterisk since the # is sent immediately after answering the call. It may be that the test was failing due to the blind transfer never actually occurring. I would suggest waiting for a Bridge event from Asterisk and then issuing an AMI redirect at that point. That eliminates some of the potential randomness from the test and would simplify the dialplan.
> > 
> > 2) The file size check. It seems like a very unsophisticated way to determine if the AUDIOHOOK_INHERIT worked as expected. We have tools like test_events now that can be inserted into the code to tell for sure whether mixmonitor has stopped or whether an audiohook has been moved as expected. We didn't have that back when this test was originally written. Something more explicit than a file size check would likely make this test more stable.
> > 
> > Finally, I disapprove of the removal of the debugging messages that previously existed in this test. Those sort of messages are very helpful when trying to find out where or potentially why a test run has failed.