[Piglit] [PATCH v2 04/10] GL3.2 GL_ARB_sync: Basic test for DeleteSync

[Chad Versace]

On 08/20/2013 11:26 AM, Nicholas Mack wrote:
> v2: Fix comments, add test for passing invalid sync to IsSync(), change variable
>      types.
> ---
>   tests/spec/arb_sync/CMakeLists.gl.txt |  2 +-
>   tests/spec/arb_sync/DeleteSync.c      | 71 +++++++++++++++++++++++++++++++++++
>   2 files changed, 72 insertions(+), 1 deletion(-)
>   create mode 100644 tests/spec/arb_sync/DeleteSync.c


The test needs to be added to all.tests.

> diff --git a/tests/spec/arb_sync/DeleteSync.c b/tests/spec/arb_sync/DeleteSync.c
> new file mode 100644
> index 0000000..71305ee
> --- /dev/null
> +++ b/tests/spec/arb_sync/DeleteSync.c



> +/**


As I mentioned in patch 1, at the top of this comment should be a \file command.


> + * Test DeleteSync() returns correct error messages
> + *
> + * Section 5.2(Sync Objects and Fences) on p243 of OpenGL 3.2 Core says:
> + * "DeleteSync will silently ignore a sync value of zero. An INVALID_VALUE
> + *  error is generated if sync is neither zero nor the name of a sync object."
> + *
> + */
> +
> +#include "piglit-util-gl-common.h"
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> +
> +config.supports_gl_compat_version = 10;


All lines inside the CONFIG_BEGIN block should be indented.


> +
> +PIGLIT_GL_TEST_CONFIG_END
> +
> +enum piglit_result
> +piglit_display(void)
> +{
> +	/* UNREACHED */
> +	return PIGLIT_FAIL;
> +}
> +
> +void
> +piglit_init(int argc, char **argv)
> +{
> +	bool pass = true;
> +	GLsync test;


The variable 'test' should be renamed to 'sync' or 'fence'. The name
'test' is confusing.


> +	GLsync invalid = (GLsync) GL_FRONT;


Here, at the top of piglit_init(), the test needs to require GL 3.2 or GL_ARB_sync.


> +
> +	/* Test for successful function calls */
> +	/* DeleteSync will silently ignore a sync value of zero */


This comment needs to be reformatted into the usual Piglit style. That is,
without "*/" trailing each line. Like this:

   /* Test for stuff...
    * and more stuff...
    */


> +	glDeleteSync(0);
> +	pass = piglit_check_gl_error(GL_NO_ERROR) && pass;
> +
> +	test = glFenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0);
> +	glDeleteSync(test);
> +	pass = piglit_check_gl_error(GL_NO_ERROR) && pass;
> +	/* Check if test was deleted */
> +	pass = !glIsSync(test) && pass;
> +
> +	/* Test for unsuccessful function calls */
> +	glDeleteSync(invalid);
> +	pass = piglit_check_gl_error(GL_INVALID_VALUE) && pass;
> +
> +	piglit_report_result(pass ? PIGLIT_PASS : PIGLIT_FAIL);
> +}

Other than the above issues, the test looks correct.