[Piglit] [PATCH 18/20] framework: Improve the ExecTest run() and get_command_result() implementation.

[Jon Severinsson]

Previously, the interface between run() and get_command_result() did not
allow arbitrary information to be returned, so get_command_result()
abused the command output return variable, expecting it to be parsed by
PlainExecTest.interpretResult().  However, that did't work for subclasses
overring interpretResult() (e.g. GleanTest).

This commit changes the interface so the TestResult returned by run() is
created in get_command_result() and passed to run() as a return value.
This allows get_command_result() to pass on any information available
in a structured manner.

As the process return code are now returned as part of the TestResult
dictionary rather than as a separate variable, the interpretResult()
interface could also be simplified.
---
 framework/exectest.py  |   38 ++++++++++++++++++--------------------
 framework/gleantest.py |    4 ++--
 2 filer ändrade, 20 tillägg(+), 22 borttagningar(-)

diff --git a/framework/exectest.py b/framework/exectest.py
index 55a0753..c6a4707 100644
--- a/framework/exectest.py
+++ b/framework/exectest.py
@@ -63,7 +63,7 @@ class ExecTest(Test):
 
 		self.skip_test = self.check_for_skip_scenario(command)
 
-	def interpretResult(self, out, returncode, results):
+	def interpretResult(self, results, out):
 		raise NotImplementedError
 		return out
 
@@ -97,21 +97,18 @@ class ExecTest(Test):
 			# affecting many developers.  If we catch it
 			# happening, try just re-running the test.
 			for i in range(5):
-				(out, err, returncode) = \
-					self.get_command_result(command, fullenv)
+				results, out, err = self.get_command_result(command, fullenv)
 				if out.find("Got spurious window resize") < 0:
 					break
 
-			results = TestResult()
-			results['result'] = 'fail'
-			out = self.interpretResult(out, returncode, results)
+			out = self.interpretResult(results, out)
 
-			if returncode is None:
+			if results['returncode'] is None:
 				pass
-			elif returncode > 0:
-				results['note'] = 'Returncode was {0}'.format(returncode)
-			elif returncode < 0:
-				results['note'] = 'Terminated by signal {0}'.format(-returncode)
+			elif results['returncode'] > 0:
+				results['note'] = 'Returncode was {0}'.format(results['returncode'])
+			elif results['returncode'] < 0:
+				results['note'] = 'Terminated by signal {0}'.format(-results['returncode'])
 				results['result'] = 'crash'
 
 			if valgrind:
@@ -119,7 +116,7 @@ class ExecTest(Test):
 				# 'skip' for this valgrind test.
 				if results['result'] != 'pass':
 					results['result'] = 'skip'
-				elif returncode == 0:
+				elif results['returncode'] == 0:
 					# Test passes and is valgrind clean.
 					results['result'] = 'pass'
 				else:
@@ -132,8 +129,7 @@ class ExecTest(Test):
 			if env:
 				results['environment'] = env
 
-			results['info'] = "Returncode: {0}\n\nErrors:\n{1}\n\nOutput:\n{2}".format(returncode, err, out)
-			results['returncode'] = returncode
+			results['info'] = "Returncode: {0}\n\nErrors:\n{1}\n\nOutput:\n{2}".format(results['returncode'], err, out)
 			results['command'] = ' '.join(self.command)
 
 			self.handleErr(results, err)
@@ -151,6 +147,7 @@ class ExecTest(Test):
 
 	def get_command_result(self, command, fullenv):
 		try:
+			results = TestResult()
 			proc = subprocess.Popen(
 				command,
 				stdout=subprocess.PIPE,
@@ -159,7 +156,7 @@ class ExecTest(Test):
 				universal_newlines=True
 				)
 			out, err = proc.communicate()
-			returncode = proc.returncode
+			results['returncode'] = proc.returncode
 
 			# In Python 2.x proc.communicate() returns 8-bit strings,
 			# but we need unicode strings.  This is because we will
@@ -184,13 +181,14 @@ class ExecTest(Test):
 			# Piglit should not report that test as having
 			# failed.
 			if e.errno == errno.ENOENT:
-				out = "PIGLIT: {'result': 'skip'}\n" \
-				    + "Test executable not found.\n"
+				results['result'] = 'skip'
+				results['returncode'] = None
+				results['note'] = "Test executable not found."
+				out = ""
 				err = ""
-				returncode = None
 			else:
 				raise e
-		return out, err, returncode
+		return results, out, err
 
 #############################################################################
 ##### PlainExecTest: Run a "native" piglit test executable
@@ -203,7 +201,7 @@ class PlainExecTest(ExecTest):
 		# Prepend testBinDir to the path.
 		self.command[0] = testBinDir + self.command[0]
 
-	def interpretResult(self, out, returncode, results):
+	def interpretResult(self, results, out):
 		outlines = out.split('\n')
 		outpiglit = list(map(lambda s: s[7:], filter(lambda s: s.startswith('PIGLIT:'), outlines)))
 
diff --git a/framework/gleantest.py b/framework/gleantest.py
index 597b005..b76b649 100644
--- a/framework/gleantest.py
+++ b/framework/gleantest.py
@@ -47,9 +47,9 @@ class GleanTest(ExecTest):
                 self.command += GleanTest.globalParams
                 return ExecTest.run(self, valgrind)
 
-	def interpretResult(self, out, returncode, results):
+	def interpretResult(self, results, out):
 		if out.find('FAIL') >= 0:
 			results['result'] = 'fail'
-		else:
+		elif 'result' not in results:
 			results['result'] = 'pass'
 		return out
-- 
1.7.10.4