[Piglit] [PATCH] egl: test that EGL_BAD_PARAMETER is returned for an invalid EGLImage attrib

[Tapani Pälli]

On 01/11/2017 05:24 PM, Emil Velikov wrote:
> On 10 January 2017 at 10:51, Tapani Pälli <tapani.palli at intel.com> wrote:
>> This test does not work correctly for me (skips because EGL_KHR_image_base
>> is unsupported even though driver supports it). I believe you either would
>> need to utilize egl-util framework (check the other tests in egl directory)
>> or initialize EGL manually ground up like some tests do. Or is this
>> depending on some compilation flags of Piglit?
>>
> Things are initialized. The issue is that the default/mixed_glx_egl
> heuristics are "fun".
>
> Namely: most likely on your setup GLX gets picked thus things go bonkers.
> All we need is to check that we're using EGL and bail our accordingly.
>
> The egl-utils helpers are quite dare I say it ... broken, so please
> don't add any tests that use those.

I voted to egl-utils instead of current logic as current logic is 
broken. It is true that egl-utils works only with X but currently we 
don't have other way to be sure we get EGL (with a test written for 
EGL!). We should probably start to work on changes to this so that one 
can reliably know what backend will be picked. It is crazy that EGL 
tests will be simply skipped on systems like Fedora 25.


>
>>> diff --git a/tests/all.py b/tests/all.py
>>> index 93d64e6..fdc1947 100644
>>> --- a/tests/all.py
>>> +++ b/tests/all.py
>>> @@ -4394,6 +4394,8 @@ with profile.group_manager(
>>>      g(['egl-create-largest-pbuffer-surface'],
>>>        'largest possible eglCreatePbufferSurface and then glClear',
>>>        run_concurrent=False)
>>> +    g(['egl-invalid-attr'],
>>> +      run_concurrent=False)
> Afaict, we have no reason to run this concurrently ?
>
>>>
>>>  with profile.group_manager(
>>>          PiglitGLTest,
>>> diff --git a/tests/egl/CMakeLists.gl.txt b/tests/egl/CMakeLists.gl.txt
>>> index 06fbecb..7a3eb6d 100644
>>> --- a/tests/egl/CMakeLists.gl.txt
>>> +++ b/tests/egl/CMakeLists.gl.txt
>>> @@ -27,6 +27,8 @@ IF(${CMAKE_SYSTEM_NAME} MATCHES "Linux")
>>>         target_link_libraries(egl-configless-context pthread
>>> ${X11_X11_LIB})
>>>         piglit_add_executable (egl-gl-colorspace egl-util.c
>>> egl-gl-colorspace.c)
>>>         target_link_libraries(egl-gl-colorspace pthread ${X11_X11_LIB})
>>> +       piglit_add_executable (egl-invalid-attr egl-invalid-attr.c)
>>> +       target_link_libraries(egl-invalid-attr pthread ${X11_X11_LIB})
> You're not using anything X related so you should be fine with not
> linking those ?
>
>
>>> --- /dev/null
>>> +++ b/tests/egl/egl-invalid-attr.c
>
>>> +PIGLIT_GL_TEST_CONFIG_BEGIN
>>> +
>>> +        config.supports_gl_core_version = 31;
>>> +
> There's nothing GL core 3.1 specific in the test. Stick with
> supports_gl_compat_version = 10 and polish the tex code below if
> needed.
>
>
>>> +
>>> +       if (piglit_check_egl_error(EGL_BAD_PARAMETER)) {
>>> +               result = PIGLIT_PASS;
>>> +       }
> No brackets for a single line if statements, please.
>
>
>>> +void
>>> +piglit_init(int argc, char **argv)
>>> +{
> Use eglGetCurrentDisplay() as a "do I have EGL?" check. We might want
> to make this (or something more robust) a helper in the future.
> Then check for each required extensions via piglit_require_egl_extension.
>
> -Emil
>