----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3476/#review11734 -----------------------------------------------------------
I suggest writing a sample yaml file that illustrates how this is intended to be used and explains what the default values are for the various configuration options. ./asterisk/trunk/lib/python/asterisk/memory_test_condition.py <https://reviewboard.asterisk.org/r/3476/#comment21511> memory_info is a list, not a dictionary. ./asterisk/trunk/lib/python/asterisk/memory_test_condition.py <https://reviewboard.asterisk.org/r/3476/#comment21515> This doesn't do a good job of explaining what memory_range actually is intended to be used for. ./asterisk/trunk/lib/python/asterisk/memory_test_condition.py <https://reviewboard.asterisk.org/r/3476/#comment21512> This can be trimmed down a bit. The .get method of dictionaries allows for you to specify a second argument to be used if the key cannot be found in the dictionary. self.memory_range = test_config.config.get('memoryrange', 7000000) self.allocations = test_config.config.get('allocations', []) self.both = test_config.config.get('both', False) ./asterisk/trunk/lib/python/asterisk/memory_test_condition.py <https://reviewboard.asterisk.org/r/3476/#comment21516> Since you do not override add_build_option in your subclass of TestCondition, you don't need to use super. You can just call self.add_build_option() directly since the subclass inherits all methods from the base class. ./asterisk/trunk/lib/python/asterisk/memory_test_condition.py <https://reviewboard.asterisk.org/r/3476/#comment21513> You can simply initialize this dictionary to have all the necessary values in it instead of adding them one-by-one: info = { 'name': 'Global', 'memory': memory, 'range': self.memory_range } ./asterisk/trunk/lib/python/asterisk/memory_test_condition.py <https://reviewboard.asterisk.org/r/3476/#comment21514> You can do a similar dictionary initialization here. Set tokens before initializing info: info = { 'memory': int(tokens[0]), 'name': allocation['name'], 'range': allocation.get('range', 500) } ./asterisk/trunk/lib/python/asterisk/memory_test_condition.py <https://reviewboard.asterisk.org/r/3476/#comment21517> Just call self.pass_check() instead of super. ./asterisk/trunk/lib/python/asterisk/memory_test_condition.py <https://reviewboard.asterisk.org/r/3476/#comment21518> Just use self.get_memory() instead of super ./asterisk/trunk/lib/python/asterisk/memory_test_condition.py <https://reviewboard.asterisk.org/r/3476/#comment21528> This seems like it's not going to work properly. related_test_condition.memory_info and self.memory_info are not guaranteed to have related memory allocations at the same ordinal positions in their lists. Also, it's possible for a requested allocation not to exist in related_test_condition.memory_info but to be present in self.memory_info. So I have two suggestions here. Change the loop to iterate over self.memory_info instead of related_test_condition.memory_info. Get the precondition's memory information by key. Something like the following: for post_cond in self.memory_info: pre_cond = None for item in related_test_condition.memory_info: if post_cond['name'] == item['name']: pre_cond = item break Now you have your correlated pre and post memory counts for any given allocation. There may be some spiffier way of doing the inner loop, but I don't know it. Note that you may not find a corresponding pre_cond for a given post_cond. In this case, you'll have to assume that 0 bytes of memory had previously been allocated. ./asterisk/trunk/lib/python/asterisk/memory_test_condition.py <https://reviewboard.asterisk.org/r/3476/#comment21526> I highly recommend using more descriptive names for these dictionaries than dictionary1 and dictionary2. ./asterisk/trunk/lib/python/asterisk/memory_test_condition.py <https://reviewboard.asterisk.org/r/3476/#comment21529> I'm a bit torn on the use of fabs() here. This will currently cause tests to fail if the memory usage *decreases* during the test run by the configured range. On the one hand, if the memory used goes down by that much, that's kind of a good thing since we're cleaning up after ourselves. But on the other hand, if the memory is spiking by that much, it's possible that something bad is happening. ./asterisk/trunk/lib/python/asterisk/memory_test_condition.py <https://reviewboard.asterisk.org/r/3476/#comment21519> Call self.fail_check() instead of super. ./asterisk/trunk/lib/python/asterisk/memory_test_condition.py <https://reviewboard.asterisk.org/r/3476/#comment21520> Call self.fail_check() instead of super. ./asterisk/trunk/lib/python/asterisk/memory_test_condition.py <https://reviewboard.asterisk.org/r/3476/#comment21521> Call self.pass_check() instead of super. ./asterisk/trunk/lib/python/asterisk/memory_test_condition.py <https://reviewboard.asterisk.org/r/3476/#comment21522> Call self.get_memory() instead of super. ./asterisk/trunk/lib/python/asterisk/test_conditions.py <https://reviewboard.asterisk.org/r/3476/#comment21530> Any particular reason you switched this away from raising an exception? - Mark Michelson On April 24, 2014, 7:02 p.m., Benjamin Keith Ford wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3476/ > ----------------------------------------------------------- > > (Updated April 24, 2014, 7:02 p.m.) > > > Review request for Asterisk Developers. > > > Bugs: ASTERISK-18429 > https://issues.asterisk.org/jira/browse/ASTERISK-18429 > > > Repository: testsuite > > > Description > ------- > > This testcondition can be enabled for any test using the keyword 'memory' > under testconditions. The purpose of this testcondition is to check the > memory allocated before and after the test, and make sure they are within a > certain range. If the test wants to look at something specific (such as > channel.c), then each allocation that you want to look at can also be > specified in under 'allocations'. If both the global memory and individual > allocations are to be checked by the test, that option can be enabled by > setting 'both' to value True. > > > Diffs > ----- > > ./asterisk/trunk/test-config.yaml 4944 > ./asterisk/trunk/lib/python/asterisk/test_conditions.py 4944 > ./asterisk/trunk/lib/python/asterisk/test_case.py 4944 > ./asterisk/trunk/lib/python/asterisk/memory_test_condition.py PRE-CREATION > > Diff: https://reviewboard.asterisk.org/r/3476/diff/ > > > Testing > ------- > > > Thanks, > > Benjamin Keith Ford > >
-- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
