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.
|
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.
|
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.
|
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:
|
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.
|
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:
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:
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
|
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.
|
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.
|
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.
|
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)
|
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?
|
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:
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.
|
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
|
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:
One thing that could be done in this case is to modify the following test from publish.tst:
to be something like
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?
|