bugGNU Octave - Bugs: bug #49767, publish.tst failure when OSMesa...

 
 

bug #49767: publish.tst failure when OSMesa excluded from build

Submitter:  Dan Sebald <sebald>
Submitted:  Mon 05 Dec 2016 08:18:03 AM UTC
   
 
Category:  Test Suite Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * GNU/Linux
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Tue 06 Dec 2016 09:59:16 PM UTC, comment #13: 

I pushed the simple fix to stable to avoid running publish.tst if neither OSMESA or gnuplot is available (http://hg.savannah.gnu.org/hgweb/octave/rev/7248717d456a).  I'm going to close this report and let bug #49507 stand for the larger issues.

Rik <rik5>
Group administrator
Tue 06 Dec 2016 05:12:43 AM UTC, comment #12: 

That's fine...  Yes, that's an alternative, but that too has its drawbacks.  It's saying that the publish test passed when really nothing was tested.  I'd think that "skip" is the more appropriate indicator when nothing is done.  I also thought to simply use "%!xtest" rather than "%!test" and let it fail as an XFAIL.  However, declaring it an expected failure isn't exactly right either.

Check the discussion at this report:

https://savannah.gnu.org/bugs/?49507

and let me know what you think about making the "%!test" conditionals slightly more robust to catch the skip at the front of the test.  We wouldn't need to change much, just use an "eval()" on the string rather than pick apart "HAVE_".

As a temporary fix, I'm fine with what you propose.

Dan Sebald <sebald>
Tue 06 Dec 2016 04:53:25 AM UTC, comment #11: 

Sorry to chime in after you've done so much research, but I don't think we need to go to great lengths here.

The current code fails if OSMESA is not available.  That's not really a failure of the publish function, its a failure to be able to draw offscreen.

Since we don't have a way to pass out-of-band information to the test.m function to indicate this is should be skipped, and you're concerned about reporting a failure when this happens, why not just mark the tests as passing.


%! if (ispc () || ! __have_feature__ ("OSMESA"))
%!   try
%!     graphics_toolkit ("gnuplot");
%!   catch
%!     ## The system doesn't support OSMESA or gnuplot for drawing hidden
%!     ## figures.  Just return and have test marked as passing.
%!     return;
%!   end_try_catch
%! endif



Rik <rik5>
Group administrator
Tue 06 Dec 2016 12:54:49 AM UTC, comment #10: 

Oh, I think the small change in the patch is the best alternative for now.  I tried a few things, but they all had drawbacks.

There is also this

octave:15> _have_feature_ ("ENABLE_DOCS")
ans = 1

Also, there can be multiple requirements in a

%!testif

but unfortunately there is no negation logic for that.  One could build a file with the desired test scripts and then call

test('onthefly')

but once one starts doing that the skips and printout start to make no sense.

Anyway, I'm going to copy here a diff file for what I think would work if only I knew how to define HAVE_GNUPLOT_AT_BUILD.  I see configure.ac, and the >>confdefs.h in configure, but I'm not making the connection and it isn't worth the effort:


diff --git a/test/publish/publish.tst b/test/publish/publish.tst
--- a/test/publish/publish.tst
+++ b/test/publish/publish.tst
@@ -18,14 +18,53 @@

 ## publish

-%!test
+%!testif HAVE_OSMESA
+%! tk = graphics_toolkit ();
 %! visibility = get (0, "defaultfigurevisible");
 %! toolkit = graphics_toolkit ();
 %! unwind_protect
 %!   set (0, "defaultfigurevisible", "off");
-%!   if (ispc ())
-%!     graphics_toolkit ("gnuplot");
-%!   endif
+%!   graphics_toolkit (tk);
+%!   cases = dir ("test_script*.m");
+%!   cases = strsplit (strrep ([cases.name], ".m", ".m\n"));
+%!   for i = 1:length(cases)-1
+%!     publish (cases{i});
+%!   endfor
+%!   confirm_recursive_rmdir (false, "local");
+%!   rmdir ("html", "s");
+%! unwind_protect_cleanup
+%!   set (0, "defaultfigurevisible", visibility);
+%! end_unwind_protect
+
+## grabcode
+
+%!testif HAVE_OSMESA
+%! tk = graphics_toolkit ();
+%! visibility = get (0, "defaultfigurevisible");
+%! toolkit = graphics_toolkit ();
+%! unwind_protect
+%!   set (0, "defaultfigurevisible", "off");
+%!   graphics_toolkit (tk);
+%!   publish ("test_script.m");
+%!   str1 = fileread ("test_script.m");
+%!   str2 = grabcode ("html/test_script.html");
+%!   confirm_recursive_rmdir (false, "local");
+%!   rmdir ("html", "s");
+%!   # Canonicalize strings
+%!   str1 = strjoin (deblank (strsplit (str1, "\n")), "\n");
+%!   str2 = strjoin (deblank (strsplit (str2, "\n")), "\n");
+%!   assert (hash ("md5", str1), hash ("md5", str2));
+%! unwind_protect_cleanup
+%!   set (0, "defaultfigurevisible", visibility);
+%! end_unwind_protect
+
+%!testif HAVE_GNUPLOT_AT_BUILD
+%! tk = "gnuplot";
+%! visibility = get (0, "defaultfigurevisible");
+%! toolkit = graphics_toolkit ();
+%! unwind_protect
+%!   set (0, "defaultfigurevisible", "off");
+%!   graphics_toolkit (tk);
 %!   cases = dir ("test_script*.m");
 %!   cases = strsplit (strrep ([cases.name], ".m", ".m\n"));
 %!   for i = 1:length(cases)-1
@@ -40,14 +79,13 @@

 ## grabcode

-%!test
+%!testif HAVE_GNUPLOT_AT_BUILD
+%! tk = "gnuplot";
 %! visibility = get (0, "defaultfigurevisible");
 %! toolkit = graphics_toolkit ();
 %! unwind_protect
 %!   set (0, "defaultfigurevisible", "off");
-%!   if (ispc ())
-%!     graphics_toolkit ("gnuplot");
-%!   endif
+%!   graphics_toolkit (tk);
 %!   publish ("test_script.m");
 %!   str1 = fileread ("test_script.m");
 %!   str2 = grabcode ("html/test_script.html");


Dan Sebald <sebald>
Mon 05 Dec 2016 09:53:01 PM UTC, comment #9: 

I forgot to add that by moving the runtime test _have_feature('OSMESA')_ back to the test.m skip level means that ispc() can be dropped because I assume that building in the PC environment will result in OSMESA = 0.

Dan Sebald <sebald>
Mon 05 Dec 2016 09:43:59 PM UTC, comment #8: 

graphics_toolkit("gnuplot") error will lead to a FAIL, though, won't it?

I've looked through the _run_test_suite_.m/test.m code to see when a skip occurs.  It seems to use the _have_feature_ function itself.  I assume there is some way of specifying the feature(s?) that is(are) required.

I also noticed that "GNUPLOT" is in the list of, not features, but build environment variables:


octave:10> __octave_config_info__
[snip]
        GLPK_LIBS = -lglpk
        GNUPLOT = gnuplot
        HDF5_CPPFLAGS =


I wonder if we could add a new feature variable (the list appears after the environment variable list) called

GNUPLOT_AT_BUILD = 1

since Octave is already obviously testing for gnuplot.  We can't say GNUPLOT is a feature because the system on which Octave is eventually installed may be different from build and there's a possibility that gnuplot isn't installed there.

But, GNUPLOT_AT_BUILD is useful because it tells us if gnuplot can be used for build purposes, e.g., generating plots for documentation.

With that distinction, rather than do the run-time test:


+%!   if (ispc () || ! __have_feature__ ("OSMESA"))
 %!     graphics_toolkit ("gnuplot");
 %!   endif


the publish.tst scripts could be expanded to run the included tests for both OSMESA present and for GNUPLOT_AT_BUILD present.  That is,

% HOWEVER OSMESA IS SPECIFIED FOR test.m
function publish_osmesa()
    pubtest('fltk');
end

% HOWEVER GNUPLOT_AT_BUILD IS SPECIFIED FOR test.m
function publish_osmesa()
    pubtest('gnuplot');
end

function pubtest(toolkit)
    graphics_toolkit(toolkit);
    % The two tests go here
end

whatever combination of new files, etc. are required to make that work.  Then there would be four possible tests, e.g., if OSMesa is not available at build:  2 PASS and 2 SKIP

Dan Sebald <sebald>
Mon 05 Dec 2016 07:41:14 PM UTC, comment #7: 

Don't worry about that part. If "gnuplot" is not in PATH (or if the user has set gnuplot_binary to something else that doesn't exist or is not executable), graphics_toolkit("gnuplot") will error.

Mike Miller <mtmiller>
Group Member
Mon 05 Dec 2016 07:20:36 PM UTC, comment #6: 

This makes sense Mike.  A new test result is too much of a change right now.

Would the "skip" determination need to check for gnuplot in runtime?  I'm not sure.  I believe there is no build option for leaving out the gnuplot scripts, so they are always present in a properly installed Octave.  It seems to me it is simply a case of checking whether gnuplot is installed in the OS in that case.

There should be a OS script that could simply run gnuplot and check whether that fails, e.g.,

gnuplot -e "exit"

If there is some way the Make process can issue that OS command and then THAT make script command indicates fail, that would be the case of no gnuplot.  So if Make could determine both built without OSMESA and the above system command fails, then it should skip.

Does that sound doable?  If so, I could add to the patch changes.

Dan Sebald <sebald>
Mon 05 Dec 2016 06:26:17 PM UTC, comment #5: 

Yeah I think this is not highly important, but harmless enough that it could go on stable. Patch looks good to me, untested.

Dan - there is yet no way to have a test send a result back to the test runner to indicate that it should be skipped based on a runtime check. So we could make the test be completely conditional on whether OSMesa is present, but gnuplot is a runtime test.

Maybe in Octave 4.4, we can have a way for tests to throw a certain error ID that the test runner interprets as a runtime signal that the test should be marked skipped, see bug #49507.

Mike Miller <mtmiller>
Group Member
Mon 05 Dec 2016 04:52:48 PM UTC, comment #4: 

OK.  Remembering the attachment this time.

Stable vs. dev?  I don't know.  Maybe it does sound like a stable sort of change; it is a simple correction to the test suite.

(file #39151)

Dan Sebald <sebald>
Mon 05 Dec 2016 04:43:15 PM UTC, comment #3: 

@Dan: Something went wrong with the upload and there is no patch attached.  Could you try again?

Second, should this be changed on stable and merged to dev?

Rik <rik5>
Group administrator
Mon 05 Dec 2016 08:25:42 AM UTC, comment #2: 

Attached is a commit patch that makes that change.  It passes for --without-OSMesa build.  Please try on your build.

Also note that the test suite prints this message:


Tests are most often skipped because the features they require
have been disabled.  Features are most often disabled because
they require dependencies that were not present when Octave
was built.  The configure script should have printed a summary
at the end of its run indicating which dependencies were not found.


Perhaps this test should fall under the skip category if neither OSMesa nor gnuplot are present.  If you think so, feel free to make that change and commit to the canonical version under your name.

Dan Sebald <sebald>
Mon 05 Dec 2016 08:19:01 AM UTC, comment #1: 

To which Mike responded:

Yes, I think it is worth fixing.

I think the right syntax for this is


  if (ispc () || ! __have_feature__ ("OSMESA"))


Dan Sebald <sebald>
Mon 05 Dec 2016 08:18:03 AM UTC, original submission:  

There are two "make check" failures when --without-OSMesa option is used to build Octave.  These failure don't overly concern me, but I'm just pointing them out because it seems that they technically shouldn't be failures in the sense that Octave is able to figure out that gnuplot is available for use in its place during the documentation build stage.

Here are the messages:


  publish.tst ................................................. PASS    0/2
                                                                  FAIL    2



!!!!! test failed
__osmesa_print__: support for offscreen rendering with OSMesa was unavailable or disabled when Octave was built


One thing that could be done in this case is to modify the following test from publish.tst:


%!   if (ispc ())
%!     graphics_toolkit ("gnuplot");
%!   endif


to be something like


%! if (ispc () || NO_OSMESA_AVAILABLE())
%! graphics_toolkit ("gnuplot");
%! endif


That way there wouldn't be a "make check" error unless there is no OSMesa and no gnuplot available.

Is it worth altering?  How does Octave currently determine at runtime during the plot-figure build that no OSMesa is available?

Dan Sebald <sebald>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by mtmiller (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by sebald (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-12-06 rik5 StatusPatch Submitted Fixed
        Open/ClosedOpen Closed
    2016-12-05 mtmiller StatusNone Patch Submitted
    2016-12-05 sebald Attached File- Added octave-gnuplot_when_no_mesa-djs2016dec05.patch, #39151

    Back to the top

    Powered by Savane 3.13-4b48.
    Corresponding source code