[Piglit] [PATCH v2] framework: Add mustpass option for the Vulkan CTS test runner

[Dylan Baker]

Quoting Alejandro Piñeiro (2016-11-10 07:01:03)
> This option was added mostly to being able to run the mustpass
> case list included on Vulkan CTS repository. As that file is in
> txt format, we assume that the case list will be in txt format.
> 
> That is not the case for the other deqp-xxx profiles, that uses
> a xml format. In any case, it is properly documented on the
> piglit.conf.example
> 
> v2: allow to receive a txt file with just the name of the test,
>     without any formatting
> ---
> 
> This new version is somewhat more intrusive. It allows to receive
> a txt file with just the name of the tests. But doing that
> it removed the ill-format-line check on deqp.make_profile.
> 
> The advantage of this is that we don't need to transform the
> mustpass file that cames with the vulkan cts repository.
> 
> FWIW, deqp-vk also accepts that format. So with this change,
> piglit just allows to receive the testcase list in both txt formats.
> 
>  framework/test/deqp.py | 14 ++++++++++----
>  piglit.conf.example    |  7 +++++++
>  tests/deqp_vk.py       | 10 +++++++---
>  3 files changed, 24 insertions(+), 7 deletions(-)
> 
> diff --git a/framework/test/deqp.py b/framework/test/deqp.py
> index 5b53efd..a63bea7 100644
> --- a/framework/test/deqp.py
> +++ b/framework/test/deqp.py
> @@ -78,6 +78,14 @@ def select_source(bin_, filename, mustpass, extra_args):
>          return iter_deqp_test_cases(
>              gen_caselist_txt(bin_, filename, extra_args))
>  

You're missing a newline, it should be two for top level functions.

> +def select_source_txt(bin_, filename, mustpass, extra_args):
> +    """Return either the mustpass list in txt format or the generated list."""
> +    if options.OPTIONS.deqp_mustpass:
> +        return iter_deqp_test_cases(mustpass)
> +    else:
> +        return iter_deqp_test_cases(
> +            gen_caselist_txt(bin_, filename, extra_args))
> +

I'm not sure how to do it, but I'm also not thrilled with the near duplication
of select_source function. I think I'd rather see either a keyword argument that
passes the function to use for deqp_mustpass or do file extension detection or
something that reduces the need for another function.

>  
>  def make_profile(test_list, test_class):
>      """Create a TestProfile instance."""
> @@ -146,10 +154,8 @@ def iter_deqp_test_cases(case_file):
>                  continue
>              elif line.startswith('TEST:'):
>                  yield line[len('TEST:'):].strip()
> -            else:
> -                raise exceptions.PiglitFatalError(
> -                    'deqp: {}:{}: ill-formed line'.format(case_file, i))
> -

Don't remove this newline, it was correct.

> +            else: # We assume that the line is just the name of the test
> +                yield line

I'm not sure I like this approach, it conflates two different files formats, and
it creates two functions that are nearly identical. I'll give you that this is
all kind of a mess, but I think you should create a new generator function for
the vulkan mustpass list, it's a different format.

Dylan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 455 bytes
Desc: signature
URL: <https://lists.freedesktop.org/archives/piglit/attachments/20161110/64648f63/attachment-0001.sig>