[Piglit] [PATCH] util: migrate the SUBTEST* macros to piglit-util.sh

[Jose Fonseca]

FYI, you'll need to rebase on top of 
http://cgit.freedesktop.org/piglit/commit/?id=83bc6862386b2d465879bcd372d61ec754534970 
, so that we use the standard C99 syntax for variadic macros.  You might 
need to manually update the moved code, as git might just mark it as a 
conflict.


Concerning the actual change, just a general remark:  these macros save 
typing but they make the test code harder to understand


I think there are other approaches that achieve the same amount of 
type-saving, without compromising readbility.

For example, we could have a


   // global contaning the current substest result
   // TODO: Make it a enum piglit_result
   bool current_subtest_result;
   char current_subtest_message[4096];

   piglit_subtest_begin() {
     current_subtest_result = true;
     current_subtest_message[0] = 0;
   }

   void
   piglit_subtest_check(bool condition, format, ...) {
     bool pass = piglit_check_gl_error((error));
     if (!pass) {
         // FIXME: append format+va_args to current_subtest_message.
     }
     current_subtest_result = current_subtest_result && pass;
   }

   void
   piglit_subtest_check_gl_error(error) {
      ...
   }

  piglit_subtest_end()
    piglit_report_subtest_result(current_subtest_result ? PIGLIT_PASS : 
PIGLIT_FAIL, current_subtest_message);
   }


And the test would do

     piglit_subtest_begin();
     piglit_subtest_check_gl_error(error, etc);
     piglit_subtest_check_gl_error(error, etc);
     piglit_subtest_check(cond, etc);
     piglit_subtest_end();

And this scheme could be extended to all piglit_check_* variants we need.

Jose



On 18/02/15 14:15, Martin Peres wrote:
> Laura requested me to move these macros to piglit-util. However, this is
> not the first time such a macro has been written. For example, curro
> introduced subtest() in /tests/spec/arb_shader_image_load_store/common.h.
> He said he also used another macro for another test.
>
> Not sure if my macros are generic-enough though. They however can make a
> good base for implementing most tests. They were definitely sufficient to
> implement all my DSA-related tests.
>
> Signed-off-by: Martin Peres <martin.peres at linux.intel.com>
> ---
>   tests/spec/arb_direct_state_access/dsa-utils.h | 16 -------------
>   tests/util/piglit-util.h                       | 32 ++++++++++++++++++++++++++
>   2 files changed, 32 insertions(+), 16 deletions(-)
>
> diff --git a/tests/spec/arb_direct_state_access/dsa-utils.h b/tests/spec/arb_direct_state_access/dsa-utils.h
> index 25b5524..a2f70b2 100644
> --- a/tests/spec/arb_direct_state_access/dsa-utils.h
> +++ b/tests/spec/arb_direct_state_access/dsa-utils.h
> @@ -39,22 +39,6 @@ extern "C" {
>
>   #include "piglit-util-gl.h"
>
> -#define SUBTEST(error, global, format, args...) \
> -do { \
> -	bool local = piglit_check_gl_error((error)); \
> -	global = global && local; \
> -	piglit_report_subtest_result(local ? PIGLIT_PASS : PIGLIT_FAIL, \
> -	                             (format), ##args); \
> -} while (0)
> -
> -#define SUBTESTCONDITION(condition, global, format, args...) \
> -do { \
> -	bool cond = (condition); \
> -	global = global && cond; \
> -	piglit_report_subtest_result(cond ? PIGLIT_PASS : PIGLIT_FAIL, \
> -	                             (format), ##args); \
> -} while (0)
> -
>   void dsa_init_program(void);
>
>   void dsa_texture_with_unit(GLuint);
> diff --git a/tests/util/piglit-util.h b/tests/util/piglit-util.h
> index 1c522a1..9a53964 100755
> --- a/tests/util/piglit-util.h
> +++ b/tests/util/piglit-util.h
> @@ -343,6 +343,38 @@ piglit_parse_subtest_args(int *argc, char *argv[],
>   uint64_t
>   piglit_gettid(void);
>
> +/**
> + * \brief Run a subtest checking the error code of OpenGL
> + *
> + * This macro checks if the current gl error is \a error and reports the result
> + * using  piglit_report_subtest_result, using the test name \a format. Finally,
> + * it updates the \a global variable that makes sure that the reported state of
> + * the test as a whole is false if any of the subtest failed.
> + */
> +#define SUBTEST(error, global, format, args...) \
> +do { \
> +	bool local = piglit_check_gl_error((error)); \
> +	global = global && local; \
> +	piglit_report_subtest_result(local ? PIGLIT_PASS : PIGLIT_FAIL, \
> +				     (format), ##args); \
> +} while (0)
> +
> +/**
> + * \brief Run a subtest checking a condition
> + *
> + * This macro checks if the \a condition is true and reports the result using
> + * piglit_report_subtest_result, using the test name \a format. Finally, it
> + * updates the \a global variable that makes sure that the reported state of
> + * the test as a whole is false if any of the subtest failed.
> + */
> +#define SUBTESTCONDITION(condition, global, format, args...) \
> +do { \
> +	bool cond = (condition); \
> +	global = global && cond; \
> +	piglit_report_subtest_result(cond ? PIGLIT_PASS : PIGLIT_FAIL, \
> +				     (format), ##args); \
> +} while (0)
> +
>   #ifdef __cplusplus
>   } /* end extern "C" */
>   #endif
>