[Piglit] [PATCH] deqp: (V2)Add option to run subset of external dEQP-GLES3

Wang, Shuo shuo.wang at intel.com
Mon Jan 19 00:50:39 PST 2015


Thank you Dylan, it's great you can help. You can get the source code of dEQP from Google Android Source Code, it is open source and no license issue. The URL is https://android.googlesource.com/platform/external/deqp/
dEQP was used to maintain by another company called drawElements, and now maintained by Google, I am not sure if we can send a patch to it.

For the detail issue you mentioned, I will send a V3 ASAP.

Thanks,
Shuo

-----Original Message-----
From: Dylan Baker [mailto:baker.dylan.c at gmail.com] 
Sent: Saturday, January 17, 2015 4:49 AM
To: piglit at lists.freedesktop.org
Cc: Wang, Shuo; Jin, Gordon
Subject: Re: [Piglit] [PATCH] deqp: (V2)Add option to run subset of external dEQP-GLES3

This is looking much better, I have a few more comments for you.

I don't think that you're generating the correct output, some of the tests have a number attached to the end of them, and I don't think that's what you want.

I'd like to help some more, and I have some ideas on how to make this faster, but I can't find deqp anywhere, if you could point me to them that would be great. If they're only available internally you can email them to my intel address: dylanx.c.baker at intel.com

On Friday, January 16, 2015 08:55:44 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/+/lollipop-dev/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.
> 
> V2:
> 1.Update the link of get test case list file.
> 2.Using xml.etree.cElementTree module instead of re module.
> 3.Fix some Piglit style issue
> 
> Signed-off-by: Wang Shuo <shuo.wang at intel.com>
> ---
>  piglit.conf.example |  6 ++++++
>  tests/deqp_gles3.py | 36 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 42 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..e174358 100644
> --- a/tests/deqp_gles3.py
> +++ b/tests/deqp_gles3.py
> @@ -21,6 +21,7 @@
>  import ConfigParser
>  import os
>  import subprocess
> +import xml.etree.ElementTree as ET

This should be xml.etree.cElementTree as ET cElementTree is much faster than ElementTree

>  
>  # Piglit modules
>  import framework
> @@ -28,6 +29,35 @@ from framework.profile import Test, TestProfile
>  
>  __all__ = ['profile']

Indent in python files should be four spaces:
def my_func(a_list):
    """Does stuff."""
    for x in a_list:
        print(x)

>  
> +def get_test_case (root, rootGroup, i, outputfile):
                    ^ please remove this space
  
rootGroup should be root_group

> +   """Parser the test case list of Google Android CTS,
> +   and store the test case list to dEQP-GLES3-cases.txt
> +   """
> +   for child in root:
> +    rootGroup[i] = child.get('name')
> +    i += 1
> +    if child.tag == "Test":
> +      outputfile.write('TEST: ')
> +      for j in xrange(0,i-1):
> +        outputfile.write('{}.'.format(rootGroup[j]))
> +      outputfile.write('{}\n'.format(rootGroup[i]))

It should be possible to replace the previous 5 lines with something like this (I haven't tested):
outputfile.write('TEST: {}\n'.format('.'.join(root_group))

> +    else:
> +      get_test_case(child, rootGroup, i, outputfile)
> +
> +def load_test_hierarchy (input, output):
                          ^ also remove this space
> +   """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

There is some extra whitespace after generate, please remove it.

> +   a new dEQP-GLES3-cases.txt which only contain the subset of dEQP.
> +   """
> +   tree = ET.parse(input)
> +   root = tree.getroot()
> +   rootGroup = range(0, 100000)
> +   i = 0

The approach here of setting rootGroup to a list of 100,000 items is very odd in python, lists in python are dynamically allocated, you can always call list.append() to add a new object to the end of the list.

> +   outputfile = open(output,'w')

Please add a space between , and 'w'

> +   get_test_case(root, rootGroup, i, outputfile)

Please use the with keyword:
with open(output, 'w') as f:
    get_test_caste(root, rootGroup, i, f)

This is the standard way to handle opening and closing files in python, and piglit uses this pattern extensively, using with you don't need to call close(), that's done for you automatically.

> +   outputfile.close()
> +
>  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 +80,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 +106,8 @@ def gen_caselist_txt():
>      subprocess.check_call(
>          [DEQP_GLES3_EXE, '--deqp-runmode=txt-caselist'],
>          cwd=basedir)
> +    if DEQP_MUSTPASS is not None:
> +      load_test_hierarchy(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