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
