[cairo] What does it take to get a make check to pass with the xcb target, using CAIRO_REF_DIR?

[Bryce Harrington]

On Thu, Jul 07, 2016 at 02:33:19PM -0400, darxus at chaosreigns.com wrote:
> On 06/30, Bryce Harrington wrote:
> > We do know there are certain tests with race conditions that will
> > alternate between passing and failing in two identical, sequential test
> > runs.  Those test cases you mention don't ring a bell, but if you see
> > it's the same set doing that each time, then that could be the case.  I
> > don't know what to do about those, and would not oppose disabling those
> > test cases (although I assume they were meant to test something
> > important...)
> 
> Yes, these failures are definitely indication of a bug.  The few I looked
> at looked like glitches in transparency and anti-aliasing.  So opening a
> bug pointing out what was disabled seems appropriate.

Yep.
 
> What would be the best way to disable these tests?  I tried making them
> xfail, by copying the corresponding test/output/*out.png files to
> test/reference/*xfail.png but I'm still getting FAILs, I suspect because
> the output has to exactly match the xfail.png, which I do not expect it to.

I found this comment in cairo-test.h:

 *  XFAIL -- An external failure. We believe the cairo output is perfect,                                        
 *           but an external renderer is causing gross failure.                                                  
 *           (We also use this to capture current WONTFIX issues within cairo,                                   
 *           such as overflow in internal coordinates, so as not to distract                                     
 *           us when regression testing.)                                                                        
 *                                                                                                               
 *  If no REF is given for a test, then it is assumed to be XFAIL.

So you might see if deleting the reference image would get it to pass.

However, I suspect rather than disabiling the test, XFAIL just means we
know it will fail.  Since these tests seem to fluctuate between passing
and failing, I wonder if it's just going to result in false positives?

Also, I don't think we've necessarily ruled out that these expose a
legitimate issue - marking it xfail might not be appropriate if that's
so.

> Delete them from test/Makefile.sources ?

Checking my notes, the record* tests were indeed some of the ones I was
also seeing intermittently pass and fail.  The test case itself doesn't
look that terribly exotic, though, so I hesistate to chalk it up to a
broken test.  If it is indeed down to variations in the rendering for
transparency and anti-aliasing, then I wonder if the test could be
tweaked to either not use it or to be less sensitive in checking it.

I poked around a bit in the test runner code.  If we do want to disable
the test, another option to actually deleting it might be to have the
test routine, or the test's preamble, return one of the error codes.
There is CAIRO_TEST_XFAILURE, although only a couple tests use that
(path-precision.c and in-fill-empty-trapezoid.c).  CAIRO_TEST_UNTESTED
and CAIRO_TEST_ERROR are more widely used.  Again though, the proper
course of action would be to rule out that there is a legitimate issue
at stake first.  Then, second, identify whether it is caused by an
external factor.

> Removing these test from test/Makefile.sources allows make check to pass
> (with an updated set of references):  record1414x.c record2x.c record90.c
> recordflip.c record.c text-rotate.c

Updating the references makes an assumption that all of the changes
being flagged are fixes rather than regressions.  When I last studied
the test output, I couldn't justify that to myself, but examining each
case and manually updating the references looked like a daunting task.
 
> Would a patch for this be accepted?  Should it be different than the one
> below?

It would be gratifying to ditch the tests, but it wouldn't be the proper
thing to do at this stage when the issue hasn't been root-caused (afaik).

Bryce

> Subject: [PATCH] Remove tests which fail differently each time.
> 
> ---
>  test/Makefile.sources | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/test/Makefile.sources b/test/Makefile.sources
> index 479b2ca..ac8ecd3 100644
> --- a/test/Makefile.sources
> +++ b/test/Makefile.sources
> @@ -266,11 +266,6 @@ test_sources = \
>  	random-intersections-curves-eo.c		\
>  	random-intersections-curves-nz.c		\
>  	raster-source.c					\
> -	record.c					\
> -	record1414x.c					\
> -	record2x.c					\
> -	record90.c					\
> -	recordflip.c					\
>  	record-extend.c					\
>  	record-neg-extents.c                            \
>  	record-mesh.c					\
> @@ -352,7 +347,6 @@ test_sources = \
>  	text-cache-crash.c				\
>  	text-glyph-range.c				\
>  	text-pattern.c					\
> -	text-rotate.c					\
>  	text-transform.c				\
>  	text-unhinted-metrics.c				\
>  	text-zero-len.c					\
> @@ -388,6 +382,14 @@ test_sources = \
>  	zero-alpha.c					\
>  	zero-mask.c
>  
> +# Disabled because they fail differently each time:
> +#	record1414x.c					\
> +	record2x.c					\
> +	record90.c					\
> +	recordflip.c					\
> +	record.c					\
> +	text-rotate.c					
> +
>  pthread_test_sources =					\
>  	pthread-same-source.c				\
>  	pthread-show-text.c				\
> -- 
> 2.7.4