On Tue, May 14, 2013 at 06:44:29PM -0700, Chad Versace wrote:
> On 05/03/2013 04:26 AM, Topi Pohjolainen wrote:
> >Signed-off-by: Topi Pohjolainen <[email protected]>
> >---
> >  .../ext_image_dma_buf_import/CMakeLists.gles2.txt  |  14 ++
> >  .../ext_image_dma_buf_import/sample_argb8888.c     | 217 
> > +++++++++++++++++++++
> >  2 files changed, 231 insertions(+)
> >  create mode 100644 tests/spec/ext_image_dma_buf_import/CMakeLists.gles2.txt
> >  create mode 100644 tests/spec/ext_image_dma_buf_import/sample_argb8888.c
> 
> Individually, the three following tests overall look good. I don't see any 
> problem with
> any one.
> 
> However, the tests duplicate a lot of code. When several tests in a family 
> all share
> similar code, what is typically done in Piglit is create a single test 
> executable with
> a command line parameters that chooses the subtest. For a good example, see
> depthstencil-render-miplevels.c

I fully agree and in fact I already refactored it. I noticed Eric's helper
for compiling simple shader programs, and while revisioning my own tests I
thought better sharing almost all the logic between the tests.

> 
> Ok, that concludes my review for v5. It looks like the series is nearing 
> completion
> and should be ready for committing soon.
_______________________________________________
Piglit mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/piglit

Reply via email to