[2/2] glx: add test for GLX_ARB_create_context_no_error

Submitted by Adam Jackson on Feb. 7, 2019, 6:49 p.m.

Details

Message ID 20190207184909.25136-3-ajax@redhat.com
State New
Headers show
Series "Add tests for *_create_context_no_error" ( rev: 1 ) in Piglit

Not browsing as part of any series.

Commit Message

Adam Jackson Feb. 7, 2019, 6:49 p.m.
From: Grigori Goronzy <greg@chown.ath.cx>

---
 .../glx_arb_create_context/CMakeLists.gl.txt  |   1 +
 tests/spec/glx_arb_create_context/common.h    |   1 +
 tests/spec/glx_arb_create_context/no-error.c  | 159 ++++++++++++++++++
 3 files changed, 161 insertions(+)
 create mode 100644 tests/spec/glx_arb_create_context/no-error.c

Patch hide | download patch | download mbox

diff --git a/tests/spec/glx_arb_create_context/CMakeLists.gl.txt b/tests/spec/glx_arb_create_context/CMakeLists.gl.txt
index f5c5e7cbc..91a87f95d 100644
--- a/tests/spec/glx_arb_create_context/CMakeLists.gl.txt
+++ b/tests/spec/glx_arb_create_context/CMakeLists.gl.txt
@@ -41,6 +41,7 @@  IF(PIGLIT_BUILD_GLX_TESTS)
 	piglit_add_executable (glx-create-context-valid-attribute-empty valid-attribute-empty.c common.c)
 	piglit_add_executable (glx-create-context-valid-attribute-null valid-attribute-null.c common.c)
 	piglit_add_executable (glx-create-context-valid-flag-forward-compatible valid-flag-forward-compatible.c common.c)
+	piglit_add_executable (glx-create-context-no-error no-error.c common.c)
 ENDIF(PIGLIT_BUILD_GLX_TESTS)
 
 # vim: ft=cmake:
diff --git a/tests/spec/glx_arb_create_context/common.h b/tests/spec/glx_arb_create_context/common.h
index 2e58b16db..e1ac28e6f 100644
--- a/tests/spec/glx_arb_create_context/common.h
+++ b/tests/spec/glx_arb_create_context/common.h
@@ -30,6 +30,7 @@  extern GLXFBConfig fbconfig;
 extern XVisualInfo *visinfo;
 extern Window win;
 extern GLXWindow glxWin;
+extern int glx_error_code;
 
 extern bool parse_version_string(const char *string, int *major, int *minor);
 
diff --git a/tests/spec/glx_arb_create_context/no-error.c b/tests/spec/glx_arb_create_context/no-error.c
new file mode 100644
index 000000000..d3b7fa470
--- /dev/null
+++ b/tests/spec/glx_arb_create_context/no-error.c
@@ -0,0 +1,159 @@ 
+/* Copyright 2017 Grigori Goronzy <greg@chown.ath.cx>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+#include "piglit-util.h"
+#include "piglit-util-gl.h"
+#include "piglit-glx-util.h"
+#include "common.h"
+
+#define BOOLSTR(x) ((x) ? "yes" : "no")
+
+static void
+fold_results(enum piglit_result *a, enum piglit_result b)
+{
+	if (*a == PIGLIT_FAIL || b == PIGLIT_FAIL)
+		*a = PIGLIT_FAIL;
+	else if (*a == PIGLIT_PASS || b == PIGLIT_PASS)
+		*a = PIGLIT_PASS;
+	else
+		*a = PIGLIT_SKIP;
+}
+
+static enum piglit_result check_no_error(bool flag, bool debug, bool robust)
+{
+	int ctx_flags = 0;
+	ctx_flags |= debug ? GLX_CONTEXT_DEBUG_BIT_ARB : 0;
+	ctx_flags |= robust ? GLX_CONTEXT_ROBUST_ACCESS_BIT_ARB : 0;
+	const int attribs[] = {
+		GLX_CONTEXT_MAJOR_VERSION_ARB, 2,
+		GLX_CONTEXT_MINOR_VERSION_ARB, 0,
+		GLX_CONTEXT_OPENGL_NO_ERROR_ARB, flag,
+		GLX_CONTEXT_FLAGS_ARB, ctx_flags,
+		None
+	};
+	static bool is_dispatch_init = false;
+	GLXContext ctx;
+	enum piglit_result pass = PIGLIT_SKIP;
+
+	printf("info: no_error=%s, debug=%s, robustness=%s\n",
+	       BOOLSTR(flag), BOOLSTR(debug), BOOLSTR(robust));
+
+	ctx = glXCreateContextAttribsARB(dpy, fbconfig, NULL, True, attribs);
+	XSync(dpy, 0);
+
+	if (ctx == NULL) {
+		if (glx_error_code != 0) {
+			if ((debug || robust) && flag) {
+				/* KHR_no_error doesn't allow the no error mode to be enabled
+				 * with KHR_debug or ARB_robustness, so context creation is
+				 * expected to fail in these cases.
+				 */
+				printf("info: context creation failed (expected)\n");
+				pass = PIGLIT_PASS;
+				goto done;
+			}
+
+			/* Most likely the API/version is not supported. */
+			pass = PIGLIT_SKIP;
+			goto done;
+		}
+
+		printf("error: context creation failed\n");
+		pass = PIGLIT_FAIL;
+		goto done;
+	}
+
+	if (!glXMakeContextCurrent(dpy, glxWin, glxWin, ctx)) {
+		printf("error: created OpenGL context, but could not make it "
+		       "current\n");
+		pass = PIGLIT_FAIL;
+		goto done;
+	}
+
+	if (!is_dispatch_init) {
+		/* We must postpone initialization of piglit-dispatch until
+		 * a context is current.
+		 */
+		piglit_dispatch_default_init(PIGLIT_DISPATCH_GL);
+		is_dispatch_init = true;
+	}
+
+	if (!piglit_is_extension_supported("GL_KHR_no_error")) {
+		printf("error: context does not report GL_KHR_no_error "
+		       "availability\n");
+		pass = PIGLIT_FAIL;
+		goto done;
+	}
+
+	GLint context_flags = 0;
+	glGetIntegerv(GL_CONTEXT_FLAGS, &context_flags);
+	if (flag && !(context_flags & GL_CONTEXT_FLAG_NO_ERROR_BIT_KHR)) {
+		printf("error: context does not have GL_KHR_no_error enabled\n");
+		pass = PIGLIT_FAIL;
+		goto done;
+	}
+
+	/* The number of texture units is a small, unsigned number. Craft an
+	 * illegal call by using a very large number that should fail on any
+	 * OpenGL implementation in practice.
+	 */
+	glActiveTexture(-1);
+	if (glGetError() != 0 && flag) {
+		printf("error: error observed with KHR_no_error enabled\n");
+		pass = PIGLIT_FAIL;
+		goto done;
+	}
+
+	pass = PIGLIT_PASS;
+done:
+	printf("info: %s\n", piglit_result_to_string(pass));
+	glXMakeContextCurrent(dpy, None, None, NULL);
+	glXDestroyContext(dpy, ctx);
+	return pass;
+}
+
+int main(int argc, char **argv)
+{
+	enum piglit_result pass = PIGLIT_SKIP;
+
+	GLX_ARB_create_context_setup();
+	piglit_require_glx_extension(dpy, "GLX_ARB_create_context_no_error");
+
+	/* "Control group": Check that errors are indeed generated without
+	 * KHR_no_error enabled. */
+	fold_results(&pass, check_no_error(false, false, false));
+	fold_results(&pass, check_no_error(false, false, false));
+	fold_results(&pass, check_no_error(false, true, false));
+	fold_results(&pass, check_no_error(false, true, false));
+
+	/* Check that KHR_no_error gets enabled and its interaction with debug and
+	 * robustness context flags. */
+	fold_results(&pass, check_no_error(true, false, false));
+	fold_results(&pass, check_no_error(true, false, false));
+	fold_results(&pass, check_no_error(true, true, false));
+	fold_results(&pass, check_no_error(true, true, false));
+	fold_results(&pass, check_no_error(true, false, true));
+	fold_results(&pass, check_no_error(true, true, true));
+
+	GLX_ARB_create_context_teardown();
+
+	piglit_report_result(pass);
+}

Comments

Adam Jackson <ajax@redhat.com> writes:

> From: Grigori Goronzy <greg@chown.ath.cx>
>
> ---
>  .../glx_arb_create_context/CMakeLists.gl.txt  |   1 +
>  tests/spec/glx_arb_create_context/common.h    |   1 +
>  tests/spec/glx_arb_create_context/no-error.c  | 159 ++++++++++++++++++
>  3 files changed, 161 insertions(+)
>  create mode 100644 tests/spec/glx_arb_create_context/no-error.c
>
> diff --git a/tests/spec/glx_arb_create_context/CMakeLists.gl.txt b/tests/spec/glx_arb_create_context/CMakeLists.gl.txt
> index f5c5e7cbc..91a87f95d 100644
> --- a/tests/spec/glx_arb_create_context/CMakeLists.gl.txt
> +++ b/tests/spec/glx_arb_create_context/CMakeLists.gl.txt
> @@ -41,6 +41,7 @@ IF(PIGLIT_BUILD_GLX_TESTS)
>  	piglit_add_executable (glx-create-context-valid-attribute-empty valid-attribute-empty.c common.c)
>  	piglit_add_executable (glx-create-context-valid-attribute-null valid-attribute-null.c common.c)
>  	piglit_add_executable (glx-create-context-valid-flag-forward-compatible valid-flag-forward-compatible.c common.c)
> +	piglit_add_executable (glx-create-context-no-error no-error.c common.c)
>  ENDIF(PIGLIT_BUILD_GLX_TESTS)
>  
>  # vim: ft=cmake:
> diff --git a/tests/spec/glx_arb_create_context/common.h b/tests/spec/glx_arb_create_context/common.h
> index 2e58b16db..e1ac28e6f 100644
> --- a/tests/spec/glx_arb_create_context/common.h
> +++ b/tests/spec/glx_arb_create_context/common.h
> @@ -30,6 +30,7 @@ extern GLXFBConfig fbconfig;
>  extern XVisualInfo *visinfo;
>  extern Window win;
>  extern GLXWindow glxWin;
> +extern int glx_error_code;
>  
>  extern bool parse_version_string(const char *string, int *major, int *minor);
>  
> diff --git a/tests/spec/glx_arb_create_context/no-error.c b/tests/spec/glx_arb_create_context/no-error.c
> new file mode 100644
> index 000000000..d3b7fa470
> --- /dev/null
> +++ b/tests/spec/glx_arb_create_context/no-error.c
> @@ -0,0 +1,159 @@
> +/* Copyright 2017 Grigori Goronzy <greg@chown.ath.cx>
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + */
> +#include "piglit-util.h"
> +#include "piglit-util-gl.h"
> +#include "piglit-glx-util.h"
> +#include "common.h"
> +
> +#define BOOLSTR(x) ((x) ? "yes" : "no")
> +
> +static void
> +fold_results(enum piglit_result *a, enum piglit_result b)
> +{
> +	if (*a == PIGLIT_FAIL || b == PIGLIT_FAIL)
> +		*a = PIGLIT_FAIL;
> +	else if (*a == PIGLIT_PASS || b == PIGLIT_PASS)
> +		*a = PIGLIT_PASS;
> +	else
> +		*a = PIGLIT_SKIP;
> +}

This is just piglit_merge_result()

> +static enum piglit_result check_no_error(bool flag, bool debug, bool robust)
> +{
> +	int ctx_flags = 0;
> +	ctx_flags |= debug ? GLX_CONTEXT_DEBUG_BIT_ARB : 0;
> +	ctx_flags |= robust ? GLX_CONTEXT_ROBUST_ACCESS_BIT_ARB : 0;
> +	const int attribs[] = {
> +		GLX_CONTEXT_MAJOR_VERSION_ARB, 2,
> +		GLX_CONTEXT_MINOR_VERSION_ARB, 0,
> +		GLX_CONTEXT_OPENGL_NO_ERROR_ARB, flag,
> +		GLX_CONTEXT_FLAGS_ARB, ctx_flags,
> +		None
> +	};
> +	static bool is_dispatch_init = false;
> +	GLXContext ctx;
> +	enum piglit_result pass = PIGLIT_SKIP;
> +
> +	printf("info: no_error=%s, debug=%s, robustness=%s\n",
> +	       BOOLSTR(flag), BOOLSTR(debug), BOOLSTR(robust));
> +
> +	ctx = glXCreateContextAttribsARB(dpy, fbconfig, NULL, True, attribs);
> +	XSync(dpy, 0);

Needs to check that we have robusness or debug and skip if they're
missing.

> +
> +	if (ctx == NULL) {
> +		if (glx_error_code != 0) {
> +			if ((debug || robust) && flag) {
> +				/* KHR_no_error doesn't allow the no error mode to be enabled
> +				 * with KHR_debug or ARB_robustness, so context creation is
> +				 * expected to fail in these cases.
> +				 */
> +				printf("info: context creation failed (expected)\n");
> +				pass = PIGLIT_PASS;
> +				goto done;
> +			}
> +
> +			/* Most likely the API/version is not supported. */
> +			pass = PIGLIT_SKIP;
> +			goto done;
> +		}
> +
> +		printf("error: context creation failed\n");
> +		pass = PIGLIT_FAIL;
> +		goto done;
> +	}
> +
> +	if (!glXMakeContextCurrent(dpy, glxWin, glxWin, ctx)) {
> +		printf("error: created OpenGL context, but could not make it "
> +		       "current\n");
> +		pass = PIGLIT_FAIL;
> +		goto done;
> +	}
> +
> +	if (!is_dispatch_init) {
> +		/* We must postpone initialization of piglit-dispatch until
> +		 * a context is current.
> +		 */
> +		piglit_dispatch_default_init(PIGLIT_DISPATCH_GL);
> +		is_dispatch_init = true;
> +	}
> +
> +	if (!piglit_is_extension_supported("GL_KHR_no_error")) {
> +		printf("error: context does not report GL_KHR_no_error "
> +		       "availability\n");
> +		pass = PIGLIT_FAIL;
> +		goto done;
> +	}
> +
> +	GLint context_flags = 0;
> +	glGetIntegerv(GL_CONTEXT_FLAGS, &context_flags);
> +	if (flag && !(context_flags & GL_CONTEXT_FLAG_NO_ERROR_BIT_KHR)) {
> +		printf("error: context does not have GL_KHR_no_error enabled\n");
> +		pass = PIGLIT_FAIL;
> +		goto done;
> +	}
> +
> +	/* The number of texture units is a small, unsigned number. Craft an
> +	 * illegal call by using a very large number that should fail on any
> +	 * OpenGL implementation in practice.
> +	 */
> +	glActiveTexture(-1);
> +	if (glGetError() != 0 && flag) {
> +		printf("error: error observed with KHR_no_error enabled\n");
> +		pass = PIGLIT_FAIL;
> +		goto done;
> +	}

I'm a reluctant to trigger undefined behavior that allows anything
including application termination in a testcase.

> +
> +	pass = PIGLIT_PASS;
> +done:
> +	printf("info: %s\n", piglit_result_to_string(pass));
> +	glXMakeContextCurrent(dpy, None, None, NULL);
> +	glXDestroyContext(dpy, ctx);
> +	return pass;
> +}
> +
> +int main(int argc, char **argv)
> +{
> +	enum piglit_result pass = PIGLIT_SKIP;
> +
> +	GLX_ARB_create_context_setup();
> +	piglit_require_glx_extension(dpy, "GLX_ARB_create_context_no_error");
> +
> +	/* "Control group": Check that errors are indeed generated without
> +	 * KHR_no_error enabled. */
> +	fold_results(&pass, check_no_error(false, false, false));
> +	fold_results(&pass, check_no_error(false, false, false));
> +	fold_results(&pass, check_no_error(false, true, false));
> +	fold_results(&pass, check_no_error(false, true, false));

We don't actually verify that errors are generated without KHR_no_error
-- the checks are under if (flag).  However, the rest of piglit covers
that, so let's just delete these cases and drop the first arg to
check_no_error.

> +
> +	/* Check that KHR_no_error gets enabled and its interaction with debug and
> +	 * robustness context flags. */
> +	fold_results(&pass, check_no_error(true, false, false));
> +	fold_results(&pass, check_no_error(true, false, false));
> +	fold_results(&pass, check_no_error(true, true, false));
> +	fold_results(&pass, check_no_error(true, true, false));
> +	fold_results(&pass, check_no_error(true, false, true));
> +	fold_results(&pass, check_no_error(true, true, true));

Looks like there are 2 duplicated cases here.

> +
> +	GLX_ARB_create_context_teardown();
> +
> +	piglit_report_result(pass);
> +}
> -- 
> 2.19.1
>
> _______________________________________________
> Piglit mailing list
> Piglit@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/piglit
On Thu, 2019-02-07 at 14:53 -0800, Eric Anholt wrote:
> Adam Jackson <ajax@redhat.com> writes:
> 
> > +static void
> > +fold_results(enum piglit_result *a, enum piglit_result b)
> > +{
> > +	if (*a == PIGLIT_FAIL || b == PIGLIT_FAIL)
> > +		*a = PIGLIT_FAIL;
> > +	else if (*a == PIGLIT_PASS || b == PIGLIT_PASS)
> > +		*a = PIGLIT_PASS;
> > +	else
> > +		*a = PIGLIT_SKIP;
> > +}
> 
> This is just piglit_merge_result()

Indeed.

> > +static enum piglit_result check_no_error(bool flag, bool debug, bool robust)
> > +{
> > +	int ctx_flags = 0;
> > +	ctx_flags |= debug ? GLX_CONTEXT_DEBUG_BIT_ARB : 0;
> > +	ctx_flags |= robust ? GLX_CONTEXT_ROBUST_ACCESS_BIT_ARB : 0;
> > +	const int attribs[] = {
> > +		GLX_CONTEXT_MAJOR_VERSION_ARB, 2,
> > +		GLX_CONTEXT_MINOR_VERSION_ARB, 0,
> > +		GLX_CONTEXT_OPENGL_NO_ERROR_ARB, flag,
> > +		GLX_CONTEXT_FLAGS_ARB, ctx_flags,
> > +		None
> > +	};
> > +	static bool is_dispatch_init = false;
> > +	GLXContext ctx;
> > +	enum piglit_result pass = PIGLIT_SKIP;
> > +
> > +	printf("info: no_error=%s, debug=%s, robustness=%s\n",
> > +	       BOOLSTR(flag), BOOLSTR(debug), BOOLSTR(robust));
> > +
> > +	ctx = glXCreateContextAttribsARB(dpy, fbconfig, NULL, True, attribs);
> > +	XSync(dpy, 0);
> 
> Needs to check that we have robusness or debug and skip if they're
> missing.

There's not a separate extension string for debug, it's part of
GLX_ARB_create_context. And asking for a debug context is allowed to be
a no-op, so you can't even query the context bit afterward. True enough
about robustness though.

> > +	/* The number of texture units is a small, unsigned number. Craft an
> > +	 * illegal call by using a very large number that should fail on any
> > +	 * OpenGL implementation in practice.
> > +	 */
> > +	glActiveTexture(-1);
> > +	if (glGetError() != 0 && flag) {
> > +		printf("error: error observed with KHR_no_error enabled\n");
> > +		pass = PIGLIT_FAIL;
> > +		goto done;
> > +	}
> 
> I'm a reluctant to trigger undefined behavior that allows anything
> including application termination in a testcase.

Yeah, this seems like a mistake. Removed.

> > +int main(int argc, char **argv)
> > +{
> > +	enum piglit_result pass = PIGLIT_SKIP;
> > +
> > +	GLX_ARB_create_context_setup();
> > +	piglit_require_glx_extension(dpy, "GLX_ARB_create_context_no_error");
> > +
> > +	/* "Control group": Check that errors are indeed generated without
> > +	 * KHR_no_error enabled. */
> > +	fold_results(&pass, check_no_error(false, false, false));
> > +	fold_results(&pass, check_no_error(false, false, false));
> > +	fold_results(&pass, check_no_error(false, true, false));
> > +	fold_results(&pass, check_no_error(false, true, false));
> 
> We don't actually verify that errors are generated without KHR_no_error
> -- the checks are under if (flag).  However, the rest of piglit covers
> that, so let's just delete these cases and drop the first arg to
> check_no_error.

Will do.

> > +
> > +	/* Check that KHR_no_error gets enabled and its interaction with debug and
> > +	 * robustness context flags. */
> > +	fold_results(&pass, check_no_error(true, false, false));
> > +	fold_results(&pass, check_no_error(true, false, false));
> > +	fold_results(&pass, check_no_error(true, true, false));
> > +	fold_results(&pass, check_no_error(true, true, false));
> > +	fold_results(&pass, check_no_error(true, false, true));
> > +	fold_results(&pass, check_no_error(true, true, true));
> 
> Looks like there are 2 duplicated cases here.

Will do this too.

The above review seems to apply equally to the EGL test (which also has
at least one logic error). Will resubmit both with the above feedback
addressed.

- ajax
Adam Jackson <ajax@redhat.com> writes:

> On Thu, 2019-02-07 at 14:53 -0800, Eric Anholt wrote:
>> Adam Jackson <ajax@redhat.com> writes:
>> 
>> > +static void
>> > +fold_results(enum piglit_result *a, enum piglit_result b)
>> > +{
>> > +	if (*a == PIGLIT_FAIL || b == PIGLIT_FAIL)
>> > +		*a = PIGLIT_FAIL;
>> > +	else if (*a == PIGLIT_PASS || b == PIGLIT_PASS)
>> > +		*a = PIGLIT_PASS;
>> > +	else
>> > +		*a = PIGLIT_SKIP;
>> > +}
>> 
>> This is just piglit_merge_result()
>
> Indeed.
>
>> > +static enum piglit_result check_no_error(bool flag, bool debug, bool robust)
>> > +{
>> > +	int ctx_flags = 0;
>> > +	ctx_flags |= debug ? GLX_CONTEXT_DEBUG_BIT_ARB : 0;
>> > +	ctx_flags |= robust ? GLX_CONTEXT_ROBUST_ACCESS_BIT_ARB : 0;
>> > +	const int attribs[] = {
>> > +		GLX_CONTEXT_MAJOR_VERSION_ARB, 2,
>> > +		GLX_CONTEXT_MINOR_VERSION_ARB, 0,
>> > +		GLX_CONTEXT_OPENGL_NO_ERROR_ARB, flag,
>> > +		GLX_CONTEXT_FLAGS_ARB, ctx_flags,
>> > +		None
>> > +	};
>> > +	static bool is_dispatch_init = false;
>> > +	GLXContext ctx;
>> > +	enum piglit_result pass = PIGLIT_SKIP;
>> > +
>> > +	printf("info: no_error=%s, debug=%s, robustness=%s\n",
>> > +	       BOOLSTR(flag), BOOLSTR(debug), BOOLSTR(robust));
>> > +
>> > +	ctx = glXCreateContextAttribsARB(dpy, fbconfig, NULL, True, attribs);
>> > +	XSync(dpy, 0);
>> 
>> Needs to check that we have robusness or debug and skip if they're
>> missing.
>
> There's not a separate extension string for debug, it's part of
> GLX_ARB_create_context. And asking for a debug context is allowed to be
> a no-op, so you can't even query the context bit afterward. True enough
> about robustness though.
>
>> > +	/* The number of texture units is a small, unsigned number. Craft an
>> > +	 * illegal call by using a very large number that should fail on any
>> > +	 * OpenGL implementation in practice.
>> > +	 */
>> > +	glActiveTexture(-1);
>> > +	if (glGetError() != 0 && flag) {
>> > +		printf("error: error observed with KHR_no_error enabled\n");
>> > +		pass = PIGLIT_FAIL;
>> > +		goto done;
>> > +	}
>> 
>> I'm a reluctant to trigger undefined behavior that allows anything
>> including application termination in a testcase.
>
> Yeah, this seems like a mistake. Removed.
>
>> > +int main(int argc, char **argv)
>> > +{
>> > +	enum piglit_result pass = PIGLIT_SKIP;
>> > +
>> > +	GLX_ARB_create_context_setup();
>> > +	piglit_require_glx_extension(dpy, "GLX_ARB_create_context_no_error");
>> > +
>> > +	/* "Control group": Check that errors are indeed generated without
>> > +	 * KHR_no_error enabled. */
>> > +	fold_results(&pass, check_no_error(false, false, false));
>> > +	fold_results(&pass, check_no_error(false, false, false));
>> > +	fold_results(&pass, check_no_error(false, true, false));
>> > +	fold_results(&pass, check_no_error(false, true, false));
>> 
>> We don't actually verify that errors are generated without KHR_no_error
>> -- the checks are under if (flag).  However, the rest of piglit covers
>> that, so let's just delete these cases and drop the first arg to
>> check_no_error.
>
> Will do.
>
>> > +
>> > +	/* Check that KHR_no_error gets enabled and its interaction with debug and
>> > +	 * robustness context flags. */
>> > +	fold_results(&pass, check_no_error(true, false, false));
>> > +	fold_results(&pass, check_no_error(true, false, false));
>> > +	fold_results(&pass, check_no_error(true, true, false));
>> > +	fold_results(&pass, check_no_error(true, true, false));
>> > +	fold_results(&pass, check_no_error(true, false, true));
>> > +	fold_results(&pass, check_no_error(true, true, true));
>> 
>> Looks like there are 2 duplicated cases here.
>
> Will do this too.
>
> The above review seems to apply equally to the EGL test (which also has
> at least one logic error). Will resubmit both with the above feedback
> addressed.

Sounds good!