The reason why I changed the order is because it appears that the current implementations of postprocess_iteration() and analyze_perf_constraints() assume that the test succeeded when they are called. So, in the case of a test failure, I did not want to execute those methods. If this is not the case, I don't have any issue with fixing this using your suggested approach.
On Fri, Feb 25, 2011 at 1:52 PM, Eric Li(李咏竹) <[email protected]> wrote: > I found you changed the order of calls: > > Original: > for hook in self.after_iteration_hooks: > hook(self) > self.postprocess_iteration() > self.analyze_perf_constraints(constraints) > > Your change: > try: > ... > self.postprocess_iteration() > self.analyze_perf_constraints(constraints) > finally: > for hook in self.after_iteration_hooks: > hook(self) > > > Why not: > try: > ... > finally: > for hook in self.after_iteration_hooks: > hook(self) > self.postprocess_iteration() > self.analyze_perf_constraints(constraints) > > > Less likely to introduce any regression. > > On Wed, Feb 23, 2011 at 3:30 PM, Thieu Le <[email protected]> wrote: > >> Base_test's _call_run_once() does not execute the after_iteration hooks >> if the test fails. It is important to execute the after_iteration hooks >> in the event of a test failure since these hooks may be used to gather >> important test logs to help diagnose the failure. This patch >> modifies _call_run_once() to execute after_iteration hooks regardless of >> the outcome of the test. >> >> A new unit test was added to verify the behavior of _call_run_once(). The >> patch was tested using unittest_suite.py to ensure that it did not break >> other functionalities. It was also tested with an actual after_iteration >> hook that collected test logs and the test was forced to fail. >> >> Risk: High >> Visibility: The order of when the after_iteration hooks has been changed >> and may affect existing tests. >> Bugs: None >> >> Signed-off-by: Thieu Le <[email protected]> >> --- >> client/common_lib/test.py | 35 >> +++++++++++++++++++---------------- >> client/common_lib/test_unittest.py | 24 +++++++++++++++++++++++- >> 2 files changed, 42 insertions(+), 17 deletions(-) >> >> diff --git a/client/common_lib/test.py b/client/common_lib/test.py >> index 63c1d31..c55d23b 100644 >> --- a/client/common_lib/test.py >> +++ b/client/common_lib/test.py >> @@ -198,22 +198,25 @@ class base_test(object): >> for hook in self.before_iteration_hooks: >> hook(self) >> >> - if profile_only: >> - if not self.job.profilers.present(): >> - self.job.record('WARN', None, None, 'No profilers have >> been ' >> - 'added but profile_only is set - nothing >> ' >> - 'will be run') >> - self.run_once_profiling(postprocess_profiled_run, *args, >> **dargs) >> - else: >> - self.before_run_once() >> - self.run_once(*args, **dargs) >> - self.after_run_once() >> - >> - for hook in self.after_iteration_hooks: >> - hook(self) >> + try: >> + if profile_only: >> + if not self.job.profilers.present(): >> + self.job.record('WARN', None, None, >> + 'No profilers have been added but ' >> + 'profile_only is set - nothing ' >> + 'will be run') >> + self.run_once_profiling(postprocess_profiled_run, >> + *args, **dargs) >> + else: >> + self.before_run_once() >> + self.run_once(*args, **dargs) >> + self.after_run_once() >> >> - self.postprocess_iteration() >> - self.analyze_perf_constraints(constraints) >> + self.postprocess_iteration() >> + self.analyze_perf_constraints(constraints) >> + finally: >> + for hook in self.after_iteration_hooks: >> + hook(self) >> >> >> def execute(self, iterations=None, test_length=None, >> profile_only=None, >> diff --git a/client/common_lib/test_unittest.py >> b/client/common_lib/test_unittest.py >> index 9710fd0..d2218ba 100755 >> --- a/client/common_lib/test_unittest.py >> +++ b/client/common_lib/test_unittest.py >> @@ -64,13 +64,35 @@ class Test_base_test_execute(TestTestCase): >> self.test.drop_caches_between_iterations.expect_call() >> before_hook.expect_call(self.test) >> self.test.run_once.expect_call(1, 2, arg='val') >> - after_hook.expect_call(self.test) >> self.test.postprocess_iteration.expect_call() >> self.test.analyze_perf_constraints.expect_call([]) >> + after_hook.expect_call(self.test) >> self.test._call_run_once([], False, None, (1, 2), {'arg': 'val'}) >> self.god.check_playback() >> >> >> + def test_call_run_once_with_exception(self): >> + # setup >> + self.god.stub_function(self.test, >> 'drop_caches_between_iterations') >> + self.god.stub_function(self.test, 'run_once') >> + before_hook = self.god.create_mock_function('before_hook') >> + after_hook = self.god.create_mock_function('after_hook') >> + self.test.register_before_iteration_hook(before_hook) >> + self.test.register_after_iteration_hook(after_hook) >> + error = Exception('fail') >> + >> + # tests the test._call_run_once implementation >> + self.test.drop_caches_between_iterations.expect_call() >> + before_hook.expect_call(self.test) >> + self.test.run_once.expect_call(1, 2, arg='val').and_raises(error) >> + after_hook.expect_call(self.test) >> + try: >> + self.test._call_run_once([], False, None, (1, 2), {'arg': >> 'val'}) >> + except: >> + pass >> + self.god.check_playback() >> + >> + >> def _expect_call_run_once(self): >> self.test._call_run_once.expect_call((), False, None, (), {}) >> >> -- >> 1.7.3.1 >> >> > > > -- > Eric Li > 李咏竹 > Google Kirkland > > >
_______________________________________________ Autotest mailing list [email protected] http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
