[asterisk-dev] [Code Review] Asterisk Test Suite pluggable framework (and a new CDR test)

Mark Michelson reviewboard at asterisk.org
Fri Jun 22 11:07:13 CDT 2012 

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1995/#review6552
-----------------------------------------------------------

Ship it!


This is really really neat! I'm giving a "ship it" on this because I don't see anything wrong with what you have so far. The configuration makes sense, and it's easy to see the improvement based on the changes you've implemented.

My only comments deal with how this can be augmented.

Right now, the only thing that pluggable modules can do is register a callback to be called when the test is stopping. I think there need to be more entry points for modules. The tricky bit here is that each type of test object will have to have a set of defined callbacks that modules can register. Obviously a call transfer scenario will have different logical callback points than a SIPp test where SIP peers are registered.

I think a possible easy way to allow this is to have modules register manager event callbacks with their test object. This way, modules that need to check data on a Bridge or Hangup event can be called into when that event occurs.

The stop observers are fine as written, but keep in mind that if other types of observers get added, they will likely need to be given more data either in thecallback. For instance, it may be that a SIP-related module will want to register a callback for the FullyBooted event so that it can issue the "sip set debug on" CLI command. The module will need to be given something so that it can interact with the Asterisk instance on which the event occured (such as an ami object).

All of this can be addressed at a separate time though because, as I stated earlier, I think what you have is really great and a good start for now.

- Mark


On June 20, 2012, 4:43 p.m., Matt Jordan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1995/
> -----------------------------------------------------------
> 
> (Updated June 20, 2012, 4:43 p.m.)
> 
> 
> Review request for Asterisk Developers, Mark Michelson, Paul Belanger, wdoekes, opticron, and jrose.
> 
> 
> Summary
> -------
> 
> So, this all started due to https://reviewboard.asterisk.org/r/1996.
> 
> What I wanted to do was create a batch CDR test.  I then found myself needing to extend the CDRTestCase class, as - for batch CDRs - I needed to trigger the batch creation based on 10 CDR records.  However, doing so felt problematic:
> 
> 1. The CDRTestCase class assumes you're always going to make calls to Local/1 at default.  I don't like hard-coded things, and - since I was going to need to make calls to a variety of extensions - I was going to have to make this configurable.  There is another class in the Test Suite (SimpleTestCase) that does something *very* similar, but just doesn't have the CDR logic built into it.  I could change CDRTestCase and make it do a sequence of calls - but that logic isn't specific to CDR tests.  There's lots of times where all you want the Test Suite to do is make a bunch of calls into Asterisk and tell you if any of them failed (via a UserEvent).  So this belongs more in SimpleTestCase... but then I need the CDR logic.  Drat.
> 
> 2. The CDRTestCase class - while a *vast* improvement over using just TestCase for all of the CDR tests - still feels like we're writing a bunch of the same code over and over again.  Even tests that do a bit of extra work when they check that the CDR records match (for example, the various 'Fork' tests) for the most part use the same code.
> 
> 3. It annoys me to continue to write non-.py modules and scripts, execute them as python, and write the same main(), reactor.run, ami_connect handlers, etc. for every test.  All of this violates the DRY rule, and it feels like a large amount of it could be configuration driven.
> 
> For awhile now this has been nagging me, so I decided to take a new route.  Rather then having the Test Suite execute a pre-determined filename (run-test), it would gather the information about what it was supposed to run from the YAML file.  The YAML file would determine:
>   * The Test Case object.  Currently, I'm assuming this is going to be a concrete implementation of TestCase, but it could be any object that acts as the principle actor in executing the test.
>   * Plug-ins into the Test Case object.  For the sake of CDRs, thats a small module (CDRModule) that manages the CDRs that were created and provides verification that those CDRs were created correctly (in other words, the CDR expectation logic that was in CDRTestCase).
> 
> In both types of module, Test Object or Plug-in, the instantiated objects are injected with their configuration from the YAML test config file.  Thus, their configurations can be anything that the module needs.  In the case of the Test Case objects, this was what calls to make, how many calls to make, whether or not to report errors if a call couldn't be made, etc.  In the case of the CDR Plug-In module, this was what expected CDR records to look for.
> 
> A new module, TestRunner, acts as the main entry point for all tests that are run.  It parses the YAML config file, imports the module specified in the configuration, instantiates the objects, starts the twisted reactor, and returns pass/failure to the top level run-tests.py script.
> 
> This worked well for most of the tests, and - once the infrastructure was in place - allowed me to write the batch test in about 15 minutes.  So that's nice.
> 
> As with the best laid plans, things got complex once I hit some tests that did a bit more then the CDRModule handled by default.  The Fork* CDR tests typically do a bit of extra verification on the CDR records at the end of a test run, and that verification is (a) not easily expressable in configuration, and (b) doesn't make sense to do for all other CDR tests.  As such, that logic was put into its own module, but - because its only used by three tests - it lives in the /tests/cdr directory.  In order to not have to make that folder a package (and have its own __init__.py), a custom loader was written in TestRunner that can, if told to do so, load a python module either directly from a test directory or from a provided path.  This allows a test to inject bits of logic on top of provided Plug-In modules - or provide its own specific Plug-Ins if the logic only applies to that test.
> 
> Note that *none* of this has to be used.  The current mechanisms for running a test still work just fine - you can still derive a class from TestCase, or ignore TestCase and write your own python test in run-test, or write your own bash test.  This is just another abstraction that should - hopefully - speed up test writing for those tests that have only small differences between them.
> 
> (Assuming this gets accepted, all of this will get documented up on https://wiki.asterisk.org, I promise :-))
> 
> Some future candidates for this kind of framework:
> * All SIPp tests
> * Anything using TestController
> * Basic 'core' tests that only need to establish a call into Asterisk (iax2/basic-call)
> 
> 
> Diffs
> -----
> 
>   /asterisk/trunk/lib/python/asterisk/SimpleTestCase.py 3264 
>   /asterisk/trunk/lib/python/asterisk/TestCase.py 3264 
>   /asterisk/trunk/lib/python/asterisk/TestRunner.py PRE-CREATION 
>   /asterisk/trunk/lib/python/asterisk/cdr.py 3264 
>   /asterisk/trunk/logger.conf 3264 
>   /asterisk/trunk/runtests.py 3264 
>   /asterisk/trunk/tests/cdr/ForkCdrModule.py PRE-CREATION 
>   /asterisk/trunk/tests/cdr/batch_cdrs/configs/ast1/cdr.conf PRE-CREATION 
>   /asterisk/trunk/tests/cdr/batch_cdrs/configs/ast1/extensions.conf PRE-CREATION 
>   /asterisk/trunk/tests/cdr/batch_cdrs/test-config.yaml PRE-CREATION 
>   /asterisk/trunk/tests/cdr/cdr_accountcode/run-test 3264 
>   /asterisk/trunk/tests/cdr/cdr_accountcode/test-config.yaml 3264 
>   /asterisk/trunk/tests/cdr/cdr_fork_end_time/run-test 3264 
>   /asterisk/trunk/tests/cdr/cdr_fork_end_time/test-config.yaml 3264 
>   /asterisk/trunk/tests/cdr/cdr_originate_sip_congestion_log/run-test 3264 
>   /asterisk/trunk/tests/cdr/cdr_originate_sip_congestion_log/test-config.yaml 3264 
>   /asterisk/trunk/tests/cdr/cdr_unanswered_yes/run-test 3264 
>   /asterisk/trunk/tests/cdr/cdr_unanswered_yes/test-config.yaml 3264 
>   /asterisk/trunk/tests/cdr/cdr_userfield/run-test 3264 
>   /asterisk/trunk/tests/cdr/cdr_userfield/test-config.yaml 3264 
>   /asterisk/trunk/tests/cdr/console_dial_sip_answer/run-test 3264 
>   /asterisk/trunk/tests/cdr/console_dial_sip_answer/test-config.yaml 3264 
>   /asterisk/trunk/tests/cdr/console_dial_sip_busy/run-test 3264 
>   /asterisk/trunk/tests/cdr/console_dial_sip_busy/test-config.yaml 3264 
>   /asterisk/trunk/tests/cdr/console_dial_sip_congestion/run-test 3264 
>   /asterisk/trunk/tests/cdr/console_dial_sip_congestion/test-config.yaml 3264 
>   /asterisk/trunk/tests/cdr/console_dial_sip_transfer/run-test 3264 
>   /asterisk/trunk/tests/cdr/console_dial_sip_transfer/test-config.yaml 3264 
>   /asterisk/trunk/tests/cdr/console_fork_after_busy_forward/run-test 3264 
>   /asterisk/trunk/tests/cdr/console_fork_after_busy_forward/test-config.yaml 3264 
>   /asterisk/trunk/tests/cdr/console_fork_before_dial/run-test 3264 
>   /asterisk/trunk/tests/cdr/console_fork_before_dial/test-config.yaml 3264 
>   /asterisk/trunk/tests/cdr/nocdr/run-test 3264 
>   /asterisk/trunk/tests/cdr/nocdr/test-config.yaml 3264 
>   /asterisk/trunk/tests/cdr/tests.yaml 3264 
> 
> Diff: https://reviewboard.asterisk.org/r/1995/diff
> 
> 
> Testing
> -------
> 
> Lots.  Tested that each CDR test worked before I blew away its run-test and that it worked after.  Tested that they failed as well if the CDR expectations were changed.
> 
> 
> Thanks,
> 
> Matt
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120622/c71896b6/attachment.htm>

More information about the asterisk-dev mailing list