[Piglit] [PATCH 1/7] arb_vertex_attrib_binding: Test error conditions

[Eric Anholt]

Fredrik Höglund <fredrik at kde.org> writes:

> On Tuesday 29 October 2013, Eric Anholt wrote:
>> Sorry we all dropped this on the floor.  I'm taking a look at your
>> series now.
>> 
>> Fredrik Höglund <fredrik at kde.org> writes:
>> > diff --git a/tests/spec/arb_vertex_attrib_binding/errors.c b/tests/spec/arb_vertex_attrib_binding/errors.c
>> > new file mode 100644
>> > index 0000000..adac098
>> > --- /dev/null
>> > +++ b/tests/spec/arb_vertex_attrib_binding/errors.c
>> 
>> > +#define MIN(a, b) (a < b ? a : b)
>> 
>> There's already a MIN2 macro you can use.
>
> This test doesn't actually use the macro. I guess it's a left over from the
> test I based it on.
>
>> > +	/* uncreached */
>> 
>> "unreached"
>> 
>> > +void
>> > +piglit_init(int argc, char **argv)
>> > +{
>> > +	GLint maxBindings, maxAttribs, maxRelativeOffset;
>> > +	GLuint vbo, vao;
>> > +	int i;
>> > +
>> > +	piglit_require_extension("GL_ARB_vertex_attrib_binding");
>> > +	piglit_require_extension("GL_ARB_vertex_array_bgra");
>> > +	piglit_require_extension("GL_ARB_vertex_type_2_10_10_10_rev");
>> > +	piglit_require_extension("GL_ARB_ES2_compatibility");
>> 
>> Would be nice to see the extra extensions moved into
>> piglit_is_extension_supported() checks for executing those tests.
>> Though I suppose since a lot of the code for them is expecting
>> INVALID_ENUM, you can execute that with or without the extension
>> providing them :)
>
> Yeah, I've been meaning to do that, but I wanted to hear what others had
> to say about the ARB_vertex_attrib_binding dependencies first.
>
>> > +
>> > +	/* Create and bind a vertex array object */
>> > +	glGenVertexArrays(1, &vao);
>> > +	glBindVertexArray(vao);
>> > +
>> > +	glGenBuffers(1, &vbo);
>> > +	glBindBuffer(GL_ARRAY_BUFFER, vbo);
>> > +
>> > +
>> > +	/* Query limits */
>> > +	glGetIntegerv(GL_MAX_VERTEX_ATTRIBS, &maxAttribs);
>> > +	glGetIntegerv(GL_MAX_VERTEX_ATTRIB_BINDINGS, &maxBindings);
>> > +	glGetIntegerv(GL_MAX_VERTEX_ATTRIB_RELATIVE_OFFSET, &maxRelativeOffset);
>> > +
>> > +	/* Clear the error state */
>> > +	while (glGetError() != GL_NO_ERROR) {}
>> 
>> What error state are you clearing here?  If you really do expect to have
>> something you're ignoring, piglit_reset_gl_error() is the helper for
>> this.
>
> There shouldn't be any errors at this point, it's just make sure we
> start with a clean slate.
>
> Other piglit tests seem to do this as well.

They shouldn't, if there shouldn't have been an error produced.  It's
like the extra glFinish()es some tests still have -- I haven't
successfully kept them all from sneaking in.

On the other hand, I would like to see the framework catch leftover GL
errors after test completion and throw a FAIL, but that would require
testing to fix whatever tests might leave one around currently.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20131101/6706f4cb/attachment.pgp>