Re: 3.8.0-rc1 make check failures on Fedora rawhide

[Rik]

On 12/10/2013 08:34 PM, John W. Eaton wrote:
> On 12/09/2013 06:56 PM, Rik wrote:
>> On 12/09/2013 03:39 PM, Jordi Gutiérrez Hermoso wrote:
>>> On Mon, 2013-12-09 at 15:23 -0700, Orion Poplawski wrote:
>>>   >  On 12/09/2013 09:53 AM, Rik wrote:
>>>>> On 12/08/2013 10:00 AM, address@hidden wrote:
>>>>>> Most of these are coming from the lack of DISPLAY. Shouldn't they
>>>>>> be skipped instead?
>>>>> The test suite finds that you have FLTK and proceeds to try and
>>>>> switch the graphic toolkit over to FLTK which then fails because
>>>>> no DISPLAY is set.
>>>> I'll just mention that this will be a pain for packagers. Without
>>>> jumping through a lot of hoops, there is no X environment on the
>>>> Fedora builders.
>>> More generally than for packaging, Octave should fall back to gnuplot
>>> if there is no X server. Should this patch go on the stable branch? I
>>> can prepare it.
>> Octave is already falling back to gnuplot so that behavior is okay.  The
>> problem is that the %!test is trying to switch the graphics toolkit to FLTK
>> and doesn't check if an error has occurred while doing so.
>
> I'd like to avoid unnecessary failures in the tests on systems without a
> display.
>
> What is the best way to do that?  Fix "testif HAVE_FLTK" to only succeed
> if FLTK is both available and can be used?  Allow the test to pass (or
> fail?) if the display is not available?  Something else?
>
> jwe
>
>
>
On 12/10/2013 08:34 PM, John W. Eaton wrote:
> On 12/09/2013 06:56 PM, Rik wrote:
>> On 12/09/2013 03:39 PM, Jordi Gutiérrez Hermoso wrote:
>>> On Mon, 2013-12-09 at 15:23 -0700, Orion Poplawski wrote:
>>>   >  On 12/09/2013 09:53 AM, Rik wrote:
>>>>> On 12/08/2013 10:00 AM, address@hidden wrote:
>>>>>> Most of these are coming from the lack of DISPLAY. Shouldn't they
>>>>>> be skipped instead?
>>>>> The test suite finds that you have FLTK and proceeds to try and
>>>>> switch the graphic toolkit over to FLTK which then fails because
>>>>> no DISPLAY is set.
>>>> I'll just mention that this will be a pain for packagers. Without
>>>> jumping through a lot of hoops, there is no X environment on the
>>>> Fedora builders.
>>> More generally than for packaging, Octave should fall back to gnuplot
>>> if there is no X server. Should this patch go on the stable branch? I
>>> can prepare it.
>> Octave is already falling back to gnuplot so that behavior is okay.  The
>> problem is that the %!test is trying to switch the graphics toolkit to FLTK
>> and doesn't check if an error has occurred while doing so.
>
> I'd like to avoid unnecessary failures in the tests on systems without a
display.
>
> What is the best way to do that?  Fix "testif HAVE_FLTK" to only succeed
if FLTK is both available and can be used?
I'm not so keen on this.  The implementation of %!testif is simple right
now in that it just checks whether the following string is defined
according to octave_config_info.  I'd rather not see it get cluttered up
with special tests for magic strings, like "HAVE_FLTK", which will change
in the future (HAVE_JHANDLES?, HAVE_QT_PLOTTING?).
>   Allow the test to pass (or fail?) if the display is not available? 
Something else?
The nicest would be to simply skip the test entirely, but I think it would
be acceptable to have it unconditionally pass or fail (slight preference
towards fail, because then people will at least look and triage their
failures into ones that are important and ones, like this, which aren't. 
Nobody pays attention to a test that passes and it would be giving false
confidence that Octave's graphics would work.).  Incidentally, this is
already the behavior for Octave.  When running 'test graphics_toolkit' with
no DISPLAY I get

  ***** testif HAVE_FLTK
 unwind_protect
   hf = figure ("visible", "off");
   toolkit = graphics_toolkit ();
   assert (get (0, "defaultfigure__graphics_toolkit__"), toolkit);
   graphics_toolkit (hf, "fltk");
   assert (graphics_toolkit (hf), "fltk");
 unwind_protect_cleanup
   close (hf);
 end_unwind_protect
!!!!! test failed
__init_fltk__: no graphics DISPLAY available

This implements the failed test count by one and the warning message is
pretty clear about DISPLAY being unavailable.

You could add a wordier message.  Here's an example:

  ***** testif HAVE_FLTK
 if (isempty (getenv ("DISPLAY")))
   error ("no DISPLAY available to run graphical test.  Test fails");
 endif
 unwind_protect
   hf = figure ("visible", "off");
   toolkit = graphics_toolkit ();
   assert (get (0, "defaultfigure__graphics_toolkit__"), toolkit);
   graphics_toolkit (hf, "fltk");
   assert (graphics_toolkit (hf), "fltk");
 unwind_protect_cleanup
   close (hf);
 end_unwind_protect
!!!!! test failed
no DISPLAY available to run graphical test.  Test fails

--Rik