[Piglit] [PATCH] deqp: Add option to run subset of external dEQP-GLES3
Wang, Shuo
shuo.wang at intel.com
Thu Jan 15 20:53:11 PST 2015
Thank you Dylan.
1. For the link issue. I agree with you, your link is the correct one. I generate the link since the link of dEQP is https://android.googlesource.com/platform/external/deqp, and the file is stored at deqp/android/cts/com.drawelements.deqp.gles3.xml. Since China's policy I did not try it before wrote.
2. For the es30-mustpass-2013.2.9-2014-10-13.txt issue. Since this case list is got from Google Android CTS test , and the file com.drawelements.deqp.gles3.xml is the only case list file supported in Android CTS. Although the content of es30-mustpass-2013.2.9-2014-10-13.txt is the same with com.drawelements.deqp.gles3.xml, but we should ignore it. And dEQP-GLES3-cases.txt is the certain case list name accepted by dEQP.
3. For the module re and xml.etree.cElementTree issue. Thank you for the information, I send a v2 version with subject ([PATCH] deqp: (V2)Add option to run subset of external dEQP-GLES3)to fix it.
4. For Piglit style issue. Thank you for the information, I send a v2 version with subject([PATCH] deqp: (V2)Add option to run subset of external dEQP-GLES3) to fix it.
Thanks,
Shuo
-----Original Message-----
From: Dylan Baker [mailto:baker.dylan.c at gmail.com]
Sent: Saturday, January 10, 2015 1:47 AM
To: piglit at lists.freedesktop.org
Cc: Wang, Shuo; Jin, Gordon; Chad Versace
Subject: Re: [Piglit] [PATCH] deqp: Add option to run subset of external dEQP-GLES3
I have CC'd Chad (who added dEQP support)
I have a couple of questions before I get into it:
First, your link doesn't seem to work, but I found this link (https://android.googlesource.com/platform/external/deqp/+/lollipop-dev/android/cts/),
and I'm a little confused, it might just be terminology, but I see a es30-mustpass-2013.2.9-2014-10-13.txt which on cursory examination seems to contain all of the same tests as com.drawelements.deqp.gles3.xml, already in txt form, am I incorrect?
If this text file is usable you should just remove the hardcoded use of dEQP-GLES3-cases.txt and allow different text lists to be plugged.
Second, the approach of using re's to parse xml is not good, we make extensive use of xml.etree.cElementTree to parse xml in piglit, and if we can't just rely on the text list in the source tree I would rather see a solution that uses that.
Finally, I've pointed out a few things below for your information, mostly about piglit style.
On Friday, January 09, 2015 09:26:53 PM Wang Shuo wrote:
> Google have already added a subset of dEQP into Android CTS test, and
> we believe this part of dEQP have higher priority than the rest of
> dEQP test cases.
> the case list is stored at some xml files. Such as:
> com.drawelements.deqp.gles3.xml.
> It's git repo lives in the Android tree at
> [https://android.googlesource.com/platform/external/deqp/android \
> /cts/com.drawelements.deqp.gles3.xml]
>
> This patch is based on Chad's patch(Add test profile for external
> dEQP-GLES3 tests), and add an option to run the subset of dEQP-GLES3.
> The only differnce for the running method is: you need to set the
> environment variable PIGLIT_DEQP_MUSTPASS or the piglit.conf option
> deqp-gles3.mustpasslist,then it will run the subset of dEQP follow the
> test case list. If not set, it will still run the whole dEQP.
>
> Tested on Intel HSW. There are 45866 test cases for the whole dEQP and
> 37354 test cases for the subset of dEQP using Android CTS test case
> list.
>
> Signed-off-by: Wang Shuo <shuo.wang at intel.com>
> ---
> piglit.conf.example | 6 ++++++
> tests/deqp_gles3.py | 51
> +++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 57 insertions(+)
>
> diff --git a/piglit.conf.example b/piglit.conf.example index
> b3869c2..c30eef4 100644
> --- a/piglit.conf.example
> +++ b/piglit.conf.example
> @@ -44,6 +44,12 @@ testB
> ; option is not required. The environment variable
> PIGLIT_DEQP_GLES3_EXTRA_ARGS ; overrides the value set here.
> ;extra_args=--deqp-visibility hidden
> +;
> +; Path to the test case list of CTS for deqp-gles3. You can also set
> +this with ; the environment variable PIGLIT_DEQP_MUSTPASS. Piglit
> +will run the subset of ; dEQP-GLES3 tests if and only if this option is set.
> +;mustpasslist= \
> +;
> +/android/platform/external/deqp/android/cts/com.drawelements.deqp.gle
> +s3.xml
>
> ; Section for specific oclconform test. One of these sections is
> required for ; each test list in the oclconform section and must be called:
> diff --git a/tests/deqp_gles3.py b/tests/deqp_gles3.py index
> b1bb33b..f587d06 100644
> --- a/tests/deqp_gles3.py
> +++ b/tests/deqp_gles3.py
> @@ -21,6 +21,7 @@
> import ConfigParser
> import os
> import subprocess
> +import re
>
> # Piglit modules
> import framework
> @@ -28,6 +29,50 @@ from framework.profile import Test, TestProfile
>
> __all__ = ['profile']
Piglit uses PEP8 (https://www.python.org/dev/peps/pep-0008/) naming conventions for python, functions should be all lowercase with underscores separating words. There should also be two blank lines above a function declaration and two blank lines after.
>
> +def loadTestHierarchy (input, output):
> + """Google have added a subset of dEQP to CTS test, the case list is
> + stored at some xml files. Such as: com.drawelements.deqp.gles3.xml
> + This function is used to parser the file, and generate
> + a new dEQP-GLES3-cases.txt which only contain the subset of dEQP.
> + """
> + inputfile = open(input,'r')
> + outputfile = open(output,'w')
When opening files use "with open(file, <mode>) as open_file:", this ensures that the file is closed, which these are not
> + line = inputfile.readline()
> + rootGroup = range(0, 100000)
> + i = 0
> +
> + for line in inputfile:
> + line = line.strip()
> + pat_TestSuite = re.compile(r'<TestSuite\sname="(.+?)"')
> + Name_TestSuite = re.findall(pat_TestSuite, line)
There are two things you could do here that would be better.
1) Don't put re.compile in the loop, the way it is now the re will be recompiled on each iteration of the loop, losing all of the benefit of using re.compile.
2) Generally when using re.compile you would use pat_TestSuite.findall() rather than re.findall
> + if Name_TestSuite:
> + rootGroup[i] = Name_TestSuite[0]
> + i += 1
> +
> + pat_TestCase = re.compile(r'<TestCase\sname="(.+?)"')
> + Name_TestCase = re.findall(pat_TestCase, line)
> + if Name_TestCase:
> + rootGroup[i] = Name_TestCase[0]
> + i += 1
> +
> + pat_FinishTestSuite = re.compile(r'</TestSuite')
> + Name_FinishTestSuite = re.findall(pat_FinishTestSuite, line)
> + if Name_FinishTestSuite:
> + i -= 1
> +
> + pat_FinishTestCase = re.compile(r'</TestCase')
> + Name_FinishTestCase = re.findall(pat_FinishTestCase, line)
> + if Name_FinishTestCase:
> + i -= 1
> +
> + pat_Test = re.compile(r'<Test\sname="(.+?)"')
> + Name_Test = re.findall(pat_Test, line)
> + if Name_Test:
> + outputfile.write('TEST: ')
> + for j in xrange(0,i):
> + outputfile.write('%s.' % rootGroup[j])
> + outputfile.write('%s\n' % Name_Test[0])
piglit's python uses str.format() instead of C style string replacement.
For example:
'%s\n' % 'foo'
becomes
'{}\n'.format('foo')
> +
> def get_option(env_varname, config_option):
> """Query the given environment variable and then piglit.conf for the
> option. Return None if the option is unset.
> @@ -50,6 +95,10 @@ def get_option(env_varname, config_option):
> # Path to the deqp-gles3 executable.
> DEQP_GLES3_EXE = get_option('PIGLIT_DEQP_GLES3_EXE', ('deqp-gles3',
> 'exe'))
>
> +# Path to the xml file which contained the case list of the subset of
> +dEQP # and marked as must pass in CTS.
> +DEQP_MUSTPASS = get_option('PIGLIT_DEQP_MUSTPASS', ('deqp-gles3',
> +'mustpasslist'))
> +
> DEQP_GLES3_EXTRA_ARGS = get_option('PIGLIT_DEQP_GLES3_EXTRA_ARGS',
> ('deqp-gles3', 'extra_args'))
>
> @@ -72,6 +121,8 @@ def gen_caselist_txt():
> subprocess.check_call(
> [DEQP_GLES3_EXE, '--deqp-runmode=txt-caselist'],
> cwd=basedir)
> + if DEQP_MUSTPASS is not None:
> + loadTestHierarchy(DEQP_MUSTPASS, caselist_path)
> assert os.path.exists(caselist_path)
> return caselist_path
>
> --
> 1.8.3.2
>
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
>
More information about the Piglit
mailing list