[Piglit] [PATCH piglit] egl: add test for EGL_MESA_query_driver

[Eric Engestrom]

On Tuesday, 2019-01-22 15:29:44 +0000, Eric Engestrom wrote:
> On Tuesday, 2019-01-22 13:43:07 +0000, Emil Velikov wrote:
> > Hi Eric,
> > 
> > Thanks for writing this up.
> > 
> > On Tue, 22 Jan 2019 at 12:43, Eric Engestrom <eric.engestrom at intel.com> wrote:
> > >
> > > Cc: Veluri Mithun <velurimithun38 at gmail.com>
> > > Cc: Emil Velikov <emil.l.velikov at gmail.com>
> > > Cc: Rob Clark <robdclark at gmail.com>
> > > Cc: Nicolai Hähnle <nicolai.haehnle at amd.com>
> > > Signed-off-by: Eric Engestrom <eric.engestrom at intel.com>
> > > ---
> > > The extension is currently in development in this MR:
> > > https://gitlab.freedesktop.org/mesa/mesa/merge_requests/47
> > >
> > > Veluri will send updated versions of this test if the spec changes.
> > 
> > > +
> > > +       piglit_require_egl_extension(EGL_NO_DISPLAY, "EGL_MESA_query_driver");
> > > +
> > AFAICT we need an valid/initialized display here. EGL_NO_DISPLAY is
> > for _client_ EGL extensions and EGL_MESA_query_driver is not one.
> 
> Good point, indeed.
> 
> > 
> > 
> > > +       egl_error = eglGetError();
> > > +       if (driver_name || egl_error != EGL_BAD_DISPLAY) {
> > > +               printf("eglGetDisplayDriverName() should have failed with EGL_BAD_DISPLAY\n");
> > > +               printf("Instead, it returned %s and with error %s\n",
> > > +                      driver_name, piglit_get_egl_error_name(egl_error));
> > > +               piglit_report_result(PIGLIT_FAIL);
> > > +       }
> > > +
> > This hunk seems to be an open-coded piglit_check_egl_error(), use the
> > helper instead?
> > Suggestion applies for the whole file.
> 
> Yup, didn't know about the helper :)
> 
> > 
> > > +       printf("Driver name: %s\n", driver_name);
> > > +       printf("Driver config: %s\n", driver_config);
> > > +       free(driver_config);
> > > +
> > I'd add a "TODO: add basic xml config validation/printing based on the
> > example in the spec."
> 
> That would be good in theory, but:
> - we'd need to write a proper DTD, which we don't have right now
> - xml parsing is not trivial (we'd need a library, it's not something
>   one should do by hand), and I'm not sure we want to add that kind of
>   beast to piglit.
> 
> If we decide to do this, I'm happy to write the DTD.

Actually, after thinking about this a bit more, I think we really should
have a DTD, and it should be part of the extension spec.

I've started writing it, but I don't have much time left today. I'll
send it once it's done.