[waffle] [PATCH 0/8] make check cleanup & windows support

[Emil Velikov]

On 20/08/14 00:38, Chad Versace wrote:
> On 08/04/2014 07:34 AM, Emil Velikov wrote:
>> So it seems like I've completely missed out to send this series last 
>> week, so here it goes.
>>
>> The series covers
>>  - Consolidates the duplicating gl_basic_test(s) into two 
>> groups - all and all_but_cgl (as cgl is the wierd one this time).
>>   - Adds (slightly buggy) implementation for fork() that we use on
>> POSIX platforms.
>>  - Redoes a few cmake wows so that make check/check-func works for 
>> Windows.
>>
>> The series is based on top of branch for-upstream-3.2-uncommitted-fixes
>> which as the name suggests is are (reviewed but) uncommitted fixes, which
>> Jose and Chad requested to have/land after WGL is in.
>>
>> This series is also available in the for-upstream-make-check-rework 
>> branch at my github repo.
>>
>> Please review,
>> -Emil
> 
> Hi Emil. It's good to see so much duplication removed. The series needs
> a little fix, though, because now most of the tests have the same name
> in the test output!
> 
> For example, here's a snippet of `make check-func`.
> 
> test: skip:   gl_basic.all_but_cgl_gles10
> test: skip:   gl_basic.all_but_cgl_gles11
> test: pass:   gl_basic.all_but_cgl_gles2_rgb
> test: pass:   gl_basic.all_but_cgl_gles2_rgba
> ...
> test: pass:   gl_basic.all_but_cgl_gles10
> test: pass:   gl_basic.all_but_cgl_gles11
> test: pass:   gl_basic.all_but_cgl_gles2_rgb
> test: pass:   gl_basic.all_but_cgl_gles2_rgba
> 
> The top block comes from GLX and the bottom block from Wayland. But it's
> hard to tell because of the testname collisions.
> 
> I see an easy way to fix this. I'm sure there are more I haven't thought of.
> In gl_basic_test.c, defining test functions as below should work:
> 
> TEST(gl_basic, glx_gles10) { test_all_bug_cgl_gles10(); }
> TEST(gl_basic, glx_gles11) { test_all_bug_cgl_gles11(); }
> TEST(gl_basic, glx_gles2_rgb) { test_all_bug_cgl_gles2_rgb(); }
> TEST(gl_basic, glx_gles2_rgba) { test_all_bug_cgl_gles2_rgba(); }
> // Repeat for wayland, etc.
> 
Perhaps I'm missing something but the above does not really compile.

> It does introduce some redundancy, but at least it's compact. Perhaps
> there is a more clever way to do this.
> 
Another route that I can think of is even messier - the defines

#define test_gl_basic_glx_gles10 test_gl_basic_all_but_cgl_gles10

or the one I've currently gone for(similar to yours) is to add another macro
that takes both the tests displayname and function name.

#define TEST_RUN2(group, displayname, testname) \
    wt_runner_run_test( \
           #group, #displayname, \
           test_##group##_##testname, \
           testgroup_##group##_setup, \
           testgroup_##group##_teardown)


TEST_RUN2(gl_basic, glx_gl20, all_gl20);
TEST_RUN2(gl_basic, glx_gl21, all_gl21);

Note that this still gives us a ton of duplication, yet I feel that any
further cleanups can be made at a later point :)

I have pushed the TEST_RUN2 approach in branch
'for-upstream-make-check-rework-1.3' and have tested it under Linux. Let me
know if you like the approach and it I should recent the massive mechanical
changes to the ML.

-Emil