On Thu, Apr 7, 2011 at 10:08 PM, Lucas Meneghel Rodrigues
<l...@redhat.com>wrote:
> For the KVM test, we have for a long time a system of
> 'test dependencies': If a dependency test, such as guest
> installation has failed, it is not possible to run subsequent
> tests for obvious reasons. There are minor failures though,
> that don't necessarily block other tests, such as some specific
> types of image problems found on a guest after the routine check.
>
> However, job.run_test() returns a boolean, making it not possible
> to fail a test, so we are not ignoring problems, and at the same
> time, execute dependent tests, because the failure was deemed to
> be non-fatal. Therefore, introduce and additional method,
> job.run_test_detail(), that returns the test status (see all
> possible statuses on client/common_lib/error.py), rather than
> a boolean. With this finer grained control, we can implement the
> kvm subtest dependency system in a better way.
>
> Strategy: refactor the run_test method into _run_test_base
> and creating the 2 implementations, run_test, and
> run_test_detail in terms of _run_test_base.
>
> Risk: High (touches a very very visible test writer API)
> Tests executed: run_test continues to work, run_test_detail
> provides the correct return codes and change does not break
> job_unittest.
>
This makes sense and seems like a reasonable approach. Just looking at the
diff I don't see any problems with it.
a Python style nit though:
+ (subdir, testname, group_func, timeout) = self._run_test_base(url,
+
*args,
+
**dargs)
reads easier as:
+ subdir, testname, group_func, timeout = self._run_test_base(
+ url, *args, **dargs)
to me.
-Greg
> Signed-off-by: Lucas Meneghel Rodrigues <l...@redhat.com>
> ---
> client/bin/job.py | 54
> +++++++++++++++++++++++++++++++++++++++++++++++++---
> 1 files changed, 50 insertions(+), 4 deletions(-)
>
> diff --git a/client/bin/job.py b/client/bin/job.py
> index 97412c0..9effcdb 100644
> --- a/client/bin/job.py
> +++ b/client/bin/job.py
> @@ -539,10 +539,9 @@ class base_client_job(base_job.base_job):
> raise error.UnhandledTestError(e)
>
>
> - @_run_test_complete_on_exit
> - def run_test(self, url, *args, **dargs):
> + def _run_test_base(self, url, *args, **dargs):
> """
> - Summon a test object and run it.
> + Prepares arguments and run functions to run_test and
> run_test_detail.
>
> @param url A url that identifies the test to run.
> @param tag An optional keyword argument that will be added to the
> @@ -550,7 +549,11 @@ class base_client_job(base_job.base_job):
> @param subdir_tag An optional keyword argument that will be added
> to the subdir name.
>
> - @returns True if the test passes, False otherwise.
> + @returns:
> + subdir: Test subdirectory
> + testname: Test name
> + group_func: Actual test run function
> + timeout: Test timeout
> """
> group, testname = self.pkgmgr.get_package_name(url, 'test')
> testname, subdir, tag = self._build_tagged_test_name(testname,
> dargs)
> @@ -573,6 +576,25 @@ class base_client_job(base_job.base_job):
> else:
> self.record('GOOD', subdir, testname, 'completed
> successfully')
>
> + return (subdir, testname, group_func, timeout)
> +
> +
> + @_run_test_complete_on_exit
> + def run_test(self, url, *args, **dargs):
> + """
> + Summon a test object and run it.
> +
> + @param url A url that identifies the test to run.
> + @param tag An optional keyword argument that will be added to the
> + test and subdir name.
> + @param subdir_tag An optional keyword argument that will be added
> + to the subdir name.
> +
> + @returns True if the test passes, False otherwise.
> + """
> + (subdir, testname, group_func, timeout) = self._run_test_base(url,
> +
> *args,
> +
> **dargs)
> try:
> self._rungroup(subdir, testname, group_func, timeout)
> return True
> @@ -585,6 +607,30 @@ class base_client_job(base_job.base_job):
> # UnhandledTestError that is caught above.
>
>
> + @_run_test_complete_on_exit
> + def run_test_detail(self, url, *args, **dargs):
> + """
> + Summon a test object and run it, returning test status.
> +
> + @param url A url that identifies the test to run.
> + @param tag An optional keyword argument that will be added to the
> + test and subdir name.
> + @param subdir_tag An optional keyword argument that will be added
> + to the subdir name.
> +
> + @returns Test status
> + @see: client/common_lib/error.py, exit_status
> + """
> + (subdir, testname, group_func, timeout) = self._run_test_base(url,
> +
> *args,
> +
> **dargs)
> + try:
> + self._rungroup(subdir, testname, group_func, timeout)
> + return 'GOOD'
> + except error.TestBaseException, detail:
> + return detail.exit_status
> +
> +
> def _rungroup(self, subdir, testname, function, timeout, *args,
> **dargs):
> """\
> subdir:
> --
> 1.7.4.2
>
> _______________________________________________
> Autotest mailing list
> Autotest@test.kernel.org
> http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
>
_______________________________________________
Autotest mailing list
Autotest@test.kernel.org
http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
