[Piglit] [PATCH v2 07/10] GL3.2 GL_ARB_sync: Test that IsSync returns true/false if it is given a valid/invalid sync object name

[Chad Versace]

On 08/20/2013 11:26 AM, Nicholas Mack wrote:
> v2: Fix comments, initialize variables
> ---
>   tests/spec/arb_sync/CMakeLists.gl.txt |  1 +
>   tests/spec/arb_sync/IsSync.c          | 82 +++++++++++++++++++++++++++++++++++
>   2 files changed, 83 insertions(+)
>   create mode 100644 tests/spec/arb_sync/IsSync.c


Again, the test should be added to all.tests.


> +/**


Again, a \file command is needed here.


> + * Test IsSync()
> + *
> + * Section 6.1.7(Sync Object Queries) of OpenGL 3.2 Core says:
> + * "The command
> + *	boolean IsSync( sync sync );
> + *  returns TRUE if sync is the name of a sync object. If sync is not the name
> + *  of a sync object, or if an error condition occurs, IsSync returns FALSE
> + *  (note that zero is not the name of a sync object)."


When quoting the spec like this, it's always a good idea to indent the spec quote.
That's the style used inside the Mesa code.


> + *
> + */
> +
> +#include "piglit-util-gl-common.h"
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> +
> +config.supports_gl_compat_version = 10;
> +config.supports_gl_core_version = 31;


Again, lines in the CONFIG 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 test1;
> +	GLsync test2 = (GLsync)GL_BACK;


The test would be easier to understand if the variables were renamed
'valid_sync' and 'invalid_sync'.


> +
> +	if (piglit_get_gl_version() < 32) {
> +		piglit_require_extension("GL_ARB_sync");
> +	}
> +
> +	/* Create valid sync object */
> +	test1 = glFenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0);
> +
> +	/* Check if a valid name returns true */
> +	pass = glIsSync(test1) && pass;
> +
> +	/* Check if invalid names return false */
> +	/* From the GL 3.2 Core specification:
> +	 * "If <sync> is not the name of a sync object, or if an error
> +	 *  condition occurs, IsSync returns FALSE (note that zero is not the
> +	 *  name of a sync object)."
> +	 */


The two comments above should be concatenated into one comment. And the spec
quote should be indented, as discussed above.


> +	pass = !glIsSync(test2) && pass;
> +
> +	pass = !glIsSync(0) && pass;
> +
> +	glDeleteSync(test1);
> +
> +	piglit_report_result(pass ? PIGLIT_PASS : PIGLIT_FAIL);
> +}


Other than that, the test looks correct.