[Piglit] [PATCH] tests/igt.py: add support for reading the test list from the file system

[Dylan Baker]

On Thursday, April 03, 2014 14:34:19 Thomas Wood wrote:
> On 2 April 2014 18:15, Dylan Baker <baker.dylan.c at gmail.com> wrote:
> > On Wednesday, April 02, 2014 16:12:23 Thomas Wood wrote:
> >> Signed-off-by: Thomas Wood <thomas.wood at intel.com>
> >> ---
> >> tests/igt.py | 41 +++++++++++++++++++++++++----------------
> >> 1 file changed, 25 insertions(+), 16 deletions(-)
> >> 
> >> diff --git a/tests/igt.py b/tests/igt.py
> >> index f80a6c4..4640814 100644
> >> --- a/tests/igt.py
> >> +++ b/tests/igt.py
> >> @@ -98,21 +98,30 @@ class IGTTest(ExecTest):
> >> 
> >> def listTests(listname):
> >> oldDir = os.getcwd()
> >> +
> >> + lines = None
> > 
> > I have a comment below about this.
> > 
> >> try:
> >> - os.chdir(igtTestRoot)
> >> - proc = subprocess.Popen(
> >> - ['make', listname ],
> >> - stdout=subprocess.PIPE,
> >> - stderr=subprocess.PIPE,
> >> - env=os.environ.copy(),
> >> - universal_newlines=True
> >> - )
> >> - out, err = proc.communicate()
> >> - returncode = proc.returncode
> >> - finally:
> >> - os.chdir(oldDir)
> >> -
> >> - lines = out.split('\n')
> > 
> >> + with open(path.join(igtTestRoot, listname + '.txt')) as f:
> > you should set a mode for this. Probably 'r'
> 
> 'r' is the default, but it can be specified explicitly if you prefer.
> 
> >> + lines = [ line.rstrip() for line in f.readlines() ]
> > 
> > Python convention is to never have spaces on the inside of brackets.
> > Also since you only iterate over lines once, why not use a generator:
> > lines = (l.rstrip() for l in f.readlines())
> > 
> >> + except IOError:
> >> + pass
> > 
> > rather than setting lines to None and doing an if check why not make 
the
> > block below the except IOError instead of pass, it would allow you to 
cut
> > the if check and assigning lines = None above.> 
> >> +
> >> + if lines is None:
> >> + try:
> >> + os.chdir(igtTestRoot)
> >> + proc = subprocess.Popen(
> >> + ['make', 'list-' + listname ],
> > 
> > no space inside the bracket after listname
> > 
> >> + stdout=subprocess.PIPE,
> >> + stderr=subprocess.PIPE,
> >> + env=os.environ.copy(),
> >> + universal_newlines=True
> >> + )
> >> + out, err = proc.communicate()
> >> + lines = out.split('\n')
> > 
> > suggestion: I think it's cleaner to use .splitlines() than .split('\n')
> > 
> >> + returncode = proc.returncode
> > 
> > with the way you're using Popen I'd suggest using 
subprocess.check_output,
> > a convenience wrapper for Popen: lines = 
subprocess.check_output(['make',
> > 'list-' + listname],
> > env=os.environ.copy(),
> > universal_newlnies=True).splitlines()
> > 
> > I recommend this since you don't actually use err or returncode at all.
> 
> Most of the above code has not changed since before the patch (it just
> moved into a new block). Should the changes you suggest be made in a
> separate patch?

you could do that. All of those changes were just suggestions, except for 
removing the spaces between the brackets, so if you wanted to ignore the 
rest of them you could do that too.

> 
> >> + finally:
> >> + os.chdir(oldDir)
> >> +
> >> found_header = False
> >> progs = ""
> >> 
> >> @@ -126,7 +135,7 @@ def listTests(listname):
> >> 
> >> return progs
> >> 
> >> -singleTests = listTests("list-single-tests")
> >> +singleTests = listTests("single-tests")
> >> 
> >> for test in singleTests:
> >> profile.test_list[path.join('igt', test)] = IGTTest(test)
> >> @@ -150,7 +159,7 @@ def addSubTestCases(test):
> >> profile.test_list[path.join('igt', test, subtest)] = \
> >> IGTTest(test, ['--run-subtest', subtest])
> >> 
> >> -multiTests = listTests("list-multi-tests")
> >> +multiTests = listTests("multi-tests")
> >> 
> >> for test in multiTests:
> >> addSubTestCases(test)

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20140403/d4b4f31f/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20140403/d4b4f31f/attachment-0001.sig>