Find tests in .oct files #179

cbm755 · 2018-03-10T22:42:09Z

"help meh.oct" doesn't work so strip the ".oct" if specified. This
makes doctest . work. If both meh.oct and meh.m are present,
"help meh" will get the .oct file; this does the same. doctest meh.m
will explicitly test the .m file. This may work for mex files although
I haven't tested that.

cbm755 · 2018-03-10T22:43:39Z

@mtmiller This seems to work.

I have not added tests because I don't think we want compiled code in this project.

Re: mex files: At least in Matlab, I think they are documented in a separate m-file (https://www.mathworks.com/help/matlab/matlab_external/using-help-files-with-mex-files.html). I believe that would work with this fix, but have not tested.

"help meh.oct" doesn't work so strip the ".oct" if specified. This makes `doctest .` work. If both meh.oct and meh.m are present, "help meh" will get the .oct file; this does the same. `doctest meh.m` will explicitly test the .m file. This may work for mex files although I haven't tested that. Fixes #178.

mtmiller · 2018-03-11T02:50:02Z

Yeah, works for me with my limited testing of directories of oct files.

This doesn't fix all of #178, though. It should also be possible to get a list of secondary functions defined in an oct file that has multiple DEFUNs, each with their own doc strings.

`doctest foo.oct` will test the function `foo` itself as well as any other functions defined in `foo.oct` and exposted in the autoload map. On the other hand, `doctest foo` will test only the function `foo`. Fixes #178.

cbm755 · 2018-03-12T02:46:38Z

inst/private/doctest_collect.m

+    type = 'octfile';
+  elseif (exist (what) == 3)  % .oct/.mex
+    [~, what, ~] = fileparts (what);  % strip extension if present
+    type = 'function';                % then access like any function


This block (% .oct/.mex) matches:

doctest foo (rather than doctest foo.oct). I don't think its necessary for this (a later block will DTRT) but it doesn't hurt, and maybe makes clearer the expected difference between doctest foo.oct and doctest foo.

doctest bar.mex and doctest bar: I haven't tested any .mex stuff yet.

Yes, I like this distinction between doctest func and doctest func.oct. I didn't see a clean way to do it when I was making my changes.

cbm755 · 2018-03-12T02:50:01Z

@mtmiller what do you think?

I wonder if we should make some test cases for this: they could be optional and not part of the usual test run (as they need to be compiled) but they'd be there to help anyone refactoring this code in the future.

catch22 · 2018-03-12T10:33:46Z

I agree that it would be good to have test cases that are run as part of the test suite on Travis, especially since the rules regarding which tests are picked up (and in which order) seem to be a bit subtle from what I understand.

mtmiller

Overall I think this is a much better approach than my recursive hack.

I think we can further improve it to also allow doctesting a .oct file anywhere in the load path, either like doctest foo.oct or doctest /path/to/foo.oct.

Thanks for working on this!

mtmiller · 2018-03-14T02:08:08Z

inst/private/doctest_collect.m

+    type = 'octfile';
+  elseif (exist (what) == 3)  % .oct/.mex
+    [~, what, ~] = fileparts (what);  % strip extension if present
+    type = 'function';                % then access like any function


Yes, I like this distinction between doctest func and doctest func.oct. I didn't see a clean way to do it when I was making my changes.

mtmiller · 2018-03-14T02:09:36Z

inst/private/doctest_collect.m

@@ -135,6 +141,8 @@
  targets = [target];
 elseif strcmp(type, 'class')
  targets = collect_targets_class(what, depth);
+elseif strcmp (type, 'octfile')
+  targets = collect_targets_octfile (what, depth);


This is way nicer than my recursive approach. I didn't read enough of the code to see how to build up the targets list, I just saw the way directories were handled and followed that logic. I like this a lot.

mtmiller · 2018-03-14T02:13:18Z

inst/private/doctest_collect.m

+  targets = [target];
+
+  % octfile may have many fcns in it: find them using the autoload map
+  A = fullfile (pwd, file);


This only works in the limited case where foo.oct is in the current working directory. IOW, exactly when it was called from doctest somedir and doctest already did a chdir. What if we use which or file_in_loadpath to get the full file name of [basename '.oct']?

Should we just use the match-only-the-filename approach from your PR? Maybe that's good enough for now. Do you think many people have a generic utils.oct or similar lying around if different directories?

There is some overlap here with the discussion in #87 about classes and whether only "locally" defined methods should be tested or all methods (such as those from superclasses defined elsewhere).

It's probably good enough for a start if you'd rather stay simple and just use the basename. I really don't think non-developers have their own personal .oct files floating around. The much more likely case of the same .oct file is multiple copies in different directories, maybe with a pkg load and with a manual addpath, which should be ok.

I searched the entire Debian archive for any files ending with .oct, and I found 226 unique file names, no duplicates, which doesn't say anything about third-party or personal projects, but at least no well-known projects are using names like common.oct or main.oct.

mtmiller · 2018-03-14T02:15:28Z

inst/private/doctest_collect.m

+  I = find (strcmp (files, A));
+  if (~ isempty (I))
+    % indicate that octfile has other fcns, and indent those targets
+    targets(1).name = [targets(1).name ':'];


Cool, I like the idea of labeling the function with its .oct file.

User could be testing any .oct file in the load path, so remove pwd.

cbm755 · 2018-03-19T05:27:12Z

@mtmiller I've merged your changes: are you happy with 6eb5deb?

mtmiller · 2019-01-22T18:27:08Z

Hey, sorry if you were waiting on me. This fell off my radar, but looks good to me.

cbm755 · 2019-01-22T19:42:32Z

Fell off mine too, I'll merge this for now.

Testing of this is the other PR, still not sure about how we want organize that, esp. for packagers.

cbm755 added the platform: octave label Mar 10, 2018

cbm755 force-pushed the dot_finds_oct_files branch from 74197e5 to d1e4466 Compare March 10, 2018 22:45

cbm755 added 2 commits March 11, 2018 19:36

[maint] update copyright

d3ef6c7

octfiles can contain other fcns: test those too if .oct given

0a40628

`doctest foo.oct` will test the function `foo` itself as well as any other functions defined in `foo.oct` and exposted in the autoload map. On the other hand, `doctest foo` will test only the function `foo`. Fixes #178.

cbm755 commented Mar 12, 2018

View reviewed changes

cbm755 mentioned this pull request Mar 13, 2018

[wip] autoload .oct file functions #180

Closed

mtmiller reviewed Mar 14, 2018

View reviewed changes

match only the basename of the .oct file

6eb5deb

User could be testing any .oct file in the load path, so remove pwd.

cbm755 mentioned this pull request Mar 19, 2018

Add tests for octfile support #181

Open

update doctest for oct files

7b13b8e

cbm755 merged commit 58d89d6 into master Jan 22, 2019

cbm755 deleted the dot_finds_oct_files branch January 22, 2019 19:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Find tests in .oct files #179

Find tests in .oct files #179

cbm755 commented Mar 10, 2018

cbm755 commented Mar 10, 2018

mtmiller commented Mar 11, 2018 •

edited

Loading

cbm755 Mar 12, 2018 •

edited

Loading

mtmiller Mar 14, 2018

cbm755 commented Mar 12, 2018

catch22 commented Mar 12, 2018

mtmiller left a comment

mtmiller Mar 14, 2018

mtmiller Mar 14, 2018

mtmiller Mar 14, 2018

cbm755 Mar 14, 2018

mtmiller Mar 14, 2018

mtmiller Mar 14, 2018

cbm755 commented Mar 19, 2018

mtmiller commented Jan 22, 2019

cbm755 commented Jan 22, 2019

Find tests in .oct files #179

Find tests in .oct files #179

Conversation

cbm755 commented Mar 10, 2018

cbm755 commented Mar 10, 2018

mtmiller commented Mar 11, 2018 • edited Loading

cbm755 Mar 12, 2018 • edited Loading

Choose a reason for hiding this comment

mtmiller Mar 14, 2018

Choose a reason for hiding this comment

cbm755 commented Mar 12, 2018

catch22 commented Mar 12, 2018

mtmiller left a comment

Choose a reason for hiding this comment

mtmiller Mar 14, 2018

Choose a reason for hiding this comment

mtmiller Mar 14, 2018

Choose a reason for hiding this comment

mtmiller Mar 14, 2018

Choose a reason for hiding this comment

cbm755 Mar 14, 2018

Choose a reason for hiding this comment

mtmiller Mar 14, 2018

Choose a reason for hiding this comment

mtmiller Mar 14, 2018

Choose a reason for hiding this comment

cbm755 commented Mar 19, 2018

mtmiller commented Jan 22, 2019

cbm755 commented Jan 22, 2019

mtmiller commented Mar 11, 2018 •

edited

Loading

cbm755 Mar 12, 2018 •

edited

Loading