[CRIU] [PATCH 2/2][v3] zdtm.py: add support of test reports in TAP format

[Sergey Bronnikov]

On 10:57 Tue 15 Mar , Andrew Vagin wrote:
> On Tue, Mar 15, 2016 at 12:37:09PM +0300, Pavel Emelyanov wrote:
> > I like the reply-to field in this mail :)
> > One more comment inline.
> > 
> > On 03/15/2016 11:41 AM, Sergey Bronnikov wrote:
> > > Usually we run CRIU tests automatically using Jenkins CI and it reports status
> > > as PASS/FAIL for overall testsuite on used environment. You should dig into log
> > > files to figure out how many tests were failed and skipped. This patch brings
> > > support of cute reports in TAP (Test Anything Protocol) format.
> > > 
> > > The sample of report:
> > > 
> > > TAP version 13
> > > 1..242
> > > ok 1 - conntracks # SKIP manual run only
> > > ok 2 - busyloop00
> > > ok 3 - sleeping00
> > > ok 4 - pid00
> > > ok 5 - caps00
> > > ok 6 - wait00
> 
> Can we use full names for tests? We have static/socket-tcp and
> transition/socket-tcp.

Ooops, somehow missed that we will have same tests in different groups.

> Can we report a result for each flavor?
> 
> ok 1 - h/zdtm/static/env00
> ok 2 - ns/zdtm/static/env00
> ok 3 - uns/zdtm/static/env00

For me lines in such format looks ugly and badly readable.
Why do you want reporting per flavor?
Our flavors are dependent: for example we have testcase with flavors 'h', 'ns' and
'uns'. If 'h' failed, then the rest flavors should be skipped. If 'ns' failed, it
means that 'h' passed and 'uns' was skipped. So reporting per flavor is
useless. :)

> ok 1 - h/zdtm/static/env00
> ok 2 - ns/zdtm/static/env00
> ok 3 - uns/zdtm/static/env00

> And we can add a yaml block with test output if a test failed.

Sure, will implement it in separate patch.

> > > NOTE: report will be generated only when options --keep-going and --report
> > > are specified.
> > > 
> > > Signed-off-by: Sergey Bronnikov <sergeyb at openvz.org>
> > > ---
> > >  test/zdtm.py | 38 +++++++++++++++++++++++++++++++++++++-
> > >  1 file changed, 37 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/test/zdtm.py b/test/zdtm.py
> > > index 8067e29..391292c 100755
> > > --- a/test/zdtm.py
> > > +++ b/test/zdtm.py
> > > @@ -18,6 +18,7 @@ import imp
> > >  import socket
> > >  import fcntl
> > >  import errno
> > > +import datetime
> > >  
> > >  os.chdir(os.path.dirname(os.path.abspath(__file__)))
> > >  
> > > @@ -1007,10 +1008,13 @@ class launcher:
> > >  	def __init__(self, opts, nr_tests):
> > >  		self.__opts = opts
> > >  		self.__total = nr_tests
> > > +		self.__runtest = 0
> > >  		self.__nr = 0
> > >  		self.__max = int(opts['parallel'] or 1)
> > >  		self.__subs = {}
> > >  		self.__fail = False
> > > +		self.__file_report = None
> > > +		self.__make_tap_report = False
> > 
> > You can leave only one variable -- the __file_report one -- and instead
> > of if self.__make_tap_report check for if self.__file_report.
> > 
> > >  		if self.__max > 1 and self.__total > 1:
> > >  			self.__use_log = True
> > >  		elif opts['report']:
> > > @@ -1018,6 +1022,24 @@ class launcher:
> > >  		else:
> > >  			self.__use_log = False
> > >  
> > > +		if opts['report'] and opts['keep_going']:
> > > +			self.__make_tap_report = True
> > > +
> > > +		if self.__make_tap_report:
> > > +			now = datetime.datetime.now()
> > > +			att = 0
> > > +			reportname = os.path.join(report_dir, "criu-testreport.tap")
> > > +			while os.access(reportname, os.F_OK):
> > > +				reportname = os.path.join(report_dir, "criu-testreport" + ".%d.tap" % att)
> > > +				att += 1
> 
> I think we can overwrite the old file if it exists.
> 
> > > +
> > > +			self.__file_report = open(reportname, 'a')
> > > +			print >> self.__file_report, "# Hardware architecture: " + arch
> > > +			print >> self.__file_report, "# Timestamp: " + now.strftime("%Y-%m-%d %H:%M") + " (GMT+1)"
> > > +			print >> self.__file_report, "# "
> > > +			print >> self.__file_report, "TAP version 13"
> > > +			print >> self.__file_report, "1.." + str(nr_tests)
> > > +
> > >  	def __show_progress(self):
> > >  		perc = self.__nr * 16 / self.__total
> > >  		print "=== Run %d/%d %s" % (self.__nr, self.__total, '=' * perc + '-' * (16 - perc))
> > > @@ -1025,6 +1047,10 @@ class launcher:
> > >  	def skip(self, name, reason):
> > >  		print "Skipping %s (%s)" % (name, reason)
> > >  		self.__nr += 1
> > > +		self.__runtest += 1
> > > +		if self.__make_tap_report:
> > > +			testline = "ok %d - %s # SKIP %s" % (self.__runtest, name.split('/')[-1:][0], reason)
> > > +			print >> self.__file_report, testline
> > >  
> > >  	def run_test(self, name, desc, flavor):
> > >  
> > > @@ -1059,13 +1085,21 @@ class launcher:
> > >  
> > >  	def __wait_one(self, flags):
> > >  		pid, status = os.waitpid(0, flags)
> > > +		self.__runtest += 1
> > >  		if pid != 0:
> > >  			sub = self.__subs.pop(pid)
> > >  			if status != 0:
> > > +				failed_flavor = decode_flav(os.WEXITSTATUS(status))
> > > +				if self.__file_report:
> > > +					testline = "not ok %d - %s # flavor %s" % (self.__runtest, sub['name'].split('/')[-1:][0], failed_flavor)
> > > +					print >> self.__file_report, testline
> > >  				self.__fail = True
> > >  				if sub['log']:
> > > -					failed_flavor = decode_flav(os.WEXITSTATUS(status))
> > >  					add_to_report(sub['log'], sub['name'].replace('/', '_') + "_" + failed_flavor + "/output")
> > > +			else:
> > > +				if self.__file_report:
> > > +					testline = "ok %d - %s" % (self.__runtest, sub['name'].split('/')[-1:][0])
> > > +					print >> self.__file_report, testline
> > >  
> > >  			if sub['log']:
> > >  				print open(sub['log']).read()
> > > @@ -1096,6 +1130,8 @@ class launcher:
> > >  
> > >  	def finish(self):
> > >  		self.__wait_all()
> > > +		if self.__make_tap_report:
> > > +			self.__file_report.close()
> > >  		if self.__fail:
> > >  			print_sep("FAIL", "#")
> > >  			sys.exit(1)
> > > 
> > 
> > _______________________________________________
> > CRIU mailing list
> > CRIU at openvz.org
> > https://lists.openvz.org/mailman/listinfo/criu