[CRIU] [PATCH] zdtm.py: add option --ignore-fails

[Pavel Emelyanov]

On 03/14/2016 05:14 PM, Sergey Bronnikov wrote:
> Introduce an option --ignore-fails to make ability to run all planned tests and
> ignore failed tests. Also with this option zdtm.py will keep image files and
> output log for each failed test:
> 
> [vagrant at fedora test]$ find sergeyb-xxx -maxdepth 3
> sergeyb-xxx
> sergeyb-xxx/zdtm_static_bridge_ns
> sergeyb-xxx/zdtm_static_bridge_ns/images
> sergeyb-xxx/zdtm_static_bridge_ns/images/1
> sergeyb-xxx/zdtm_static_bridge_ns/output
> sergeyb-xxx/zdtm_static_bridge_ns/images.0
> sergeyb-xxx/zdtm_static_bridge_ns/images.0/1
> sergeyb-xxx/zdtm_static_bridge_ns/output.0
> sergeyb-xxx/zdtm_static_deleted_unix_sock_h
> sergeyb-xxx/zdtm_static_deleted_unix_sock_h/output
> sergeyb-xxx/zdtm_static_deleted_unix_sock_h/output.0
> sergeyb-xxx/zdtm_static_deleted_unix_sock_h/output.1
> 
> Signed-off-by: Sergey Bronnikov <sergeyb at openvz.org>
> ---
>  test/zdtm.py | 25 ++++++++++++++++---------
>  1 file changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/test/zdtm.py b/test/zdtm.py
> index b523c62..4200f24 100755
> --- a/test/zdtm.py
> +++ b/test/zdtm.py
> @@ -76,6 +76,8 @@ def add_to_report(path, tgt_name):
>  		if os.path.isdir(path):
>  			shutil.copytree(path, tgt_path)
>  		else:
> +			if not os.path.exists(os.path.dirname(tgt_path)):
> +				os.mkdir(os.path.dirname(tgt_path))
>  			shutil.copy2(path, tgt_path)
>  
>  
> @@ -973,12 +975,15 @@ def do_run_test(tname, tdesc, flavs, opts):
>  			print_sep("Test %s FAIL at %s" % (tname, e.step), '#')
>  			t.print_output()
>  			t.kill()
> -			add_to_report(cr_api.logs(), "cr_logs")
> +			if cr_api.logs():

Why did you add this if?

> +				add_to_report(cr_api.logs(), tname.replace('/', '_') + "_" + f + "/images")
>  			if opts['keep_img'] == 'never':
>  				cr_api.cleanup()
> -			# This exit does two things -- exits from subprocess and
> -			# aborts the main script execution on the 1st error met
> -			sys.exit(1)
> +			# When option --ignore-fails not specified this exit
> +			# does two things: exits from subprocess and aborts the
> +			# main script execution on the 1st error met
> +			index = list(flavs).index(f) + 128

Please, introduce global encode_flav() and decode_flav() helpers.

> +			sys.exit(index)
>  		else:
>  			if opts['keep_img'] != 'always':
>  				cr_api.cleanup()
> @@ -1033,7 +1038,7 @@ class launcher:
>  		sub = subprocess.Popen(["./zdtm_ct", "zdtm.py"], \
>  				env = dict(os.environ, CR_CT_TEST_INFO = arg ), \
>  				stdout = log, stderr = subprocess.STDOUT)
> -		self.__subs[sub.pid] = { 'sub': sub, 'log': logf }
> +		self.__subs[sub.pid] = { 'sub': sub, 'log': logf, 'name': name, 'flavor': flavor }

You en/de-code flavor index using per-test flavort list. Why not just use the global one?

>  
>  		if test_flag(desc, 'excl'):
>  			self.wait()
> @@ -1045,7 +1050,8 @@ class launcher:
>  			if status != 0:
>  				self.__fail = True
>  				if sub['log']:
> -					add_to_report(sub['log'], "output")
> +					failed_flavor = list(sub['flavor'])[os.WEXITSTATUS(status) - 128]
> +					add_to_report(sub['log'], sub['name'].replace('/', '_') + "_" + failed_flavor + "/output")
>  
>  			if sub['log']:
>  				print open(sub['log']).read()
> @@ -1064,12 +1070,12 @@ class launcher:
>  		while self.__subs:
>  			if not self.__wait_one(os.WNOHANG):
>  				break
> -		if self.__fail:
> +		if (not opts['ignore_fails'] and self.__fail):

Maybe it's better not to set self.__fail once in case of opts['ignore_fails']?

>  			raise test_fail_exc('')
>  
>  	def wait_all(self):
>  		self.__wait_all()
> -		if self.__fail:
> +		if (not opts['ignore_fails'] and self.__fail):
>  			raise test_fail_exc('')
>  
>  	def finish(self):
> @@ -1388,7 +1394,7 @@ if os.environ.has_key('CR_CT_TEST_INFO'):
>  			wpid, status = os.wait()
>  			if wpid == pid:
>  				if not os.WIFEXITED(status) or os.WEXITSTATUS(status) != 0:

The 'or os.WEXITSTATUS(status) check becomes unneeded here.

> -					status = 1
> +					status = os.WEXITSTATUS(status)
>  				break;
>  
>  	sys.exit(status)
> @@ -1427,6 +1433,7 @@ rp.add_argument("--dry-run", help="Don't run tests, just pretend to", action='st
>  rp.add_argument("-k", "--keep-img", help = "Whether or not to keep images after test",
>  		choices = [ 'always', 'never', 'failed' ], default = 'failed')
>  rp.add_argument("--report", help = "Generate summary report in directory")
> +rp.add_argument("--ignore-fails", help = "Ignore fails and run all planned tests", action = 'store_true')

I don't insist, but Ivan's version of the option name (--keep-going) looks nicer for me.

>  
>  lp = sp.add_parser("list", help = "List tests")
>  lp.set_defaults(action = list_tests)
>