bugGNU Octave - Bugs: bug #47424, Cannot run "test...

 
 

bug #47424: Cannot run "test function" for builtin functions

Submitter:  Lachlan Andrew <lachlan>
Submitted:  Wed 16 Mar 2016 02:19:51 AM UTC
   
 
Category:  Interpreter Severity:  1 - Wish
Priority:  5 - Normal Item Group:  Feature Request
Status:  In Progress Assigned to:  lachlan
Originator Name:  Lachlan Open/Closed:  * Open
Release:  * dev Operating System:  * GNU/Linux
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Wed 23 Mar 2016 12:36:43 AM UTC, comment #5: 

Thanks for looking at this, jwe.  Your comments are, as always, spot on.

I was intending this more for developers while testing functions.  In particular, I wasn't in the habit of running the textscan BIST because running "make check" takes forever and produces voluminous output, and I couldn't just test textscan.

That developer focus is also the reason for putting the make logic in was to allow tests to be tweaked without rerunning make (and without making changes to the .cc-tst file that may not get copied back to the .cc file).  I tried just putting "make ....cc-tst" in, and it converted a seemingly instantaneous test into a multi-second wait.

Perhaps all of this suggests that there should be a separate script for testing the development version that doesn't get shipped, and a simpler extension to test.m that only checks OCTAVE_DIR/etc/tests.

Many tests test multiple functions, like the tests for  dbstatus  also testing  dbstop.  These could be tagged with a comma-separated list of functions, but it isn't clear what -tst files should be created.  We wouldn't want "make check" to run the same test multiple times.  One option would be for test itself to parse the -tst file (trivial if only we could assume awk was present...), and only run the tests for the nominated command.  That would also not require any changes to the build system.

I'm happy to make all of the changes you suggest except for changing the Makefile.  I'm happy to attempt that, but it would need careful vetting...

Lachlan Andrew <lachlan>
Tue 22 Mar 2016 02:31:56 PM UTC, comment #4: 

I agree that it would be nice to have a feature like this.  However, since multiple functions may be defined in a .oct file or a .cc file that is built in to Octave, it seems like it would be likely to mislead users into thinking that there are many more tests for built-in functions than are actually present.  I'd rather not install this change unless we have a way to tag a block of tests in a source file as belonging to a particular function and can also make the test function run only those tests for the given function, not all tests from the associated source file.

It should be relatively straightforward to tag the comment blocks with something like


%!tests-for: fcn-name
%! [...]
%!end tests-for


I don't know exactly what the syntax should be, but the point is to be able to mark a group of functions as belonging to a particular function.  Then instead of looking up the file name with "which", we could just arrange for Make to create -tst files for each function from the source files.  Then we might be able to just have the test function look for -tst files with the same name as the function.

I also don't think that we should be putting the logic of make into the test function.  Let make manage the job of rebuilding the -tst files.

The assumption that the source directory is in ".." is too specific.  The build and source trees don't have to be nested or related.

John W. Eaton <jwe>
Group administrator
Mon 21 Mar 2016 08:56:04 AM UTC, comment #3: 

Thanks for the suggestions, Mike and CarnĂ«.  The attached revision addresses them.

It should now work if the source is build in the main source directory, as well as built in .build.  (Mike, I assume the latter is what you meant by "out-of-tree build".)  It also re-makes the .cc-tst file if the .cc file is more recent.  (The make file does a grep followed by a mv, so I do the same.  Do you know why not just do the grep to the final destination?  I would have assumed the goal was an atomic replace of old by new, but the old file is removed before the grep.)  If there are no tests, this should result in an empty file, which I hope should avoid the "???? builtin" message.

If the .cc file can't be found, it uses the one in OCTAVE_HOME.  This will give the "???? builtin" message if there are no tests.  It would also be the wrong thing to do if the build is in a directory that isn't either the main source directory or a child of that.  Is there a way to test whether we're running  ./run-octave  rather than an installed version?

The has more possible code paths than the previous, and I'm not sure I've tested them all yet.  I'll confirm once I have.

(file #36710)

Lachlan Andrew <lachlan>
Fri 18 Mar 2016 11:21:24 AM UTC, comment #2: 

Very small note, instead of


exist (__name) == 5


which searches everything and may or may not return 5, we can limit the search to builtin functions right from the start:


exist (__name, "builtin")


which is also more readable.

Carnë Draug <carandraug>
Group Member
Fri 18 Mar 2016 07:02:09 AM UTC, comment #1: 

Very nice addition. This works for me in an out-of-tree build configuration.

One interesting quirk is if the file defining the builtin has no tests, the same error message is shown as before: "foo is a builtin function".

This could also be expanded later to run the tests installed in the share/octave/$ver/etc/tests directory based on the file name.

Mike Miller <mtmiller>
Group Member
Wed 16 Mar 2016 02:19:51 AM UTC, original submission:  

Trying to test an individual built in function gives something like


>> test min
????? min is a built-in function


The attached patch allows this to work when run from the build directory (either through ./run-octave or, I presume, after installing).  It simply checks if the .cc-tst file corresponding to  which min  exists, and if so, tests it.

Lachlan Andrew <lachlan>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #36710:  bug_47424_test.cset added by lachlan (3KiB - text/x-diff)
file #36661:  test_builtin.cset added by lachlan (972B - text/x-diff)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by ernstreissner
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by mtmiller (Posted a comment)
  • -email is unavailable- added by lachlan (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-08-23 ernstreissner Carbon-Copy- Added -email is unavailable-
    2016-04-14 lachlan StatusPatch Submitted In Progress
    2016-03-21 lachlan Attached File- Added bug_47424_test.cset, #36710
    2016-03-16 lachlan Attached File- Added test_builtin.cset, #36661

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code