From 45086d1c7b60c5b809ed2f104007ca29edd21843 Mon Sep 17 00:00:00 2001 From: "Colin B. Macdonald" Date: Fri, 20 Sep 2019 11:46:46 -0700 Subject: [PATCH 1/4] rename input arg to avoid shadowing builtin command "what" --- inst/doctest.m | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/inst/doctest.m b/inst/doctest.m index baf8c9f..3c1fc88 100644 --- a/inst/doctest.m +++ b/inst/doctest.m @@ -249,7 +249,7 @@ %% @seealso{test} %% @end deftypefn -function varargout = doctest(what, varargin) +function varargout = doctest(targets, varargin) %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% % Process parameters. @@ -264,8 +264,8 @@ end % if given a single object, wrap it in a cell array -if ~iscell(what) - what = {what}; +if ~iscell(targets) + targets = {targets}; end % input parsing for options and directives @@ -345,8 +345,8 @@ %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% % Collect and run tests %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% -for i=1:numel(what) - summary = doctest_collect(what{i}, directives, summary, recursive, verbose, 0, fid); +for i=1:numel(targets) + summary = doctest_collect(targets{i}, directives, summary, recursive, verbose, 0, fid); end From 0260a72f1168b2cf4db09cebad4546c9adb7bee5 Mon Sep 17 00:00:00 2001 From: "Colin B. Macdonald" Date: Fri, 20 Sep 2019 12:04:57 -0700 Subject: [PATCH 2/4] [maint] private: rename variable to avoid shadowing "what" Here "target" already has a use as a struct, so I just changed "what" into "w": short, maybe a bit ambiguous, but so was "what" ;-) --- inst/private/doctest_collect.m | 84 +++++++++++++++++----------------- 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/inst/private/doctest_collect.m b/inst/private/doctest_collect.m index 45fe698..73d8575 100644 --- a/inst/private/doctest_collect.m +++ b/inst/private/doctest_collect.m @@ -1,7 +1,7 @@ -function summary = doctest_collect(what, directives, summary, recursive, verbose, depth, fid) +function summary = doctest_collect(w, directives, summary, recursive, verbose, depth, fid) %DOCTEST_COLLECT Find and run doctests. % -% The parameter WHAT is the name of a class, directory, function or filename: +% The input W is the name of a class, directory, function or filename: % * For a directory, calls itself on the contents, recursively if % RECURSIVE is true; % * For a class, all methods are tested; @@ -21,14 +21,14 @@ % determine type of target if is_octave() - % Note: ripe for refactoring once "exist(what, 'class')" works in Octave. - [~, ~, ext] = fileparts(what); + % Note: ripe for refactoring once "exist(w, 'class')" works in Octave. + [~, ~, ext] = fileparts(w); if any(strcmpi(ext, {'.texinfo' '.texi' '.txi' '.tex'})) type = 'texinfo'; - elseif (strcmp (ext, '.oct') && exist (what) == 3) % .oct explicitly + elseif (strcmp (ext, '.oct') && exist (w) == 3) % .oct explicitly type = 'octfile'; - elseif (exist (what) == 3) % .oct/.mex - [~, what, ~] = fileparts (what); % strip extension if present + elseif (exist (w) == 3) % .oct/.mex + [~, w, ~] = fileparts (w); % strip extension if present type = 'function'; % then access like any function else type = 'unknown'; @@ -38,10 +38,10 @@ % What about classdef in oct file above? Should we do this even if % type is 'octfile'? if (strcmp (type, 'unknown')) - if (~ isempty (what) && strcmp (what(1), '@')) - temp = what(2:end); + if (~ isempty (w) && strcmp (w(1), '@')) + temp = w(2:end); else - temp = what; + temp = w; end try temp = methods(temp); @@ -52,26 +52,26 @@ end if (strcmp (type, 'unknown')) - if (exist(what, 'dir')) + if (exist(w, 'dir')) type = 'dir'; - elseif (exist(what, 'file') || exist(what, 'builtin') || exist(what) == 103) + elseif (exist(w, 'file') || exist(w, 'builtin') || exist(w) == 103) type = 'function'; else type = 'unknown'; end end else % Matlab - if (strcmp(what(1), '@')) && ~isempty(methods(what(2:end))) + if (strcmp(w(1), '@')) && ~isempty(methods(w(2:end))) % covers "doctest @class", but not "doctest @class/method" type = 'class'; - elseif ~isempty(methods(what)) + elseif ~isempty(methods(w)) % covers "doctest class" type = 'class'; - elseif (exist(what, 'dir')) + elseif (exist(w, 'dir')) type = 'dir'; - elseif exist(what, 'file') || exist(what, 'builtin'); + elseif exist(w, 'file') || exist(w, 'builtin'); type = 'function'; - elseif ~isempty(help(what)) + elseif ~isempty(help(w)) % covers "doctest class.method" and "doctest class/method" type = 'function' else @@ -85,23 +85,23 @@ % Deal with directories if (strcmp(type, 'dir')) - if (strcmp(what, '.')) + if (strcmp(w, '.')) if (depth == 0) % cheap hack to not indent when calling "doctest ." depth = -1; end else spaces = repmat(' ', 1, 2*depth); - if (strcmp(what(end), filesep())) + if (strcmp(w(end), filesep())) slashchar = ''; else slashchar = filesep(); end if (verbose) - fprintf(fid, '%s%s%s\n', spaces, what, slashchar); + fprintf(fid, '%s%s%s\n', spaces, w, slashchar); end end - oldcwd = chdir(what); + oldcwd = chdir(w); files = dir('.'); for i=1:numel(files) f = files(i).name; @@ -142,23 +142,23 @@ % TARGETS(i).depth How "deep" in a recursive traversal are we if strcmp(type, 'function') - target = collect_targets_function(what); + target = collect_targets_function(w); target.depth = depth; targets = [target]; elseif strcmp(type, 'class') - targets = collect_targets_class(what, depth); + targets = collect_targets_class(w, depth); elseif strcmp (type, 'octfile') - targets = collect_targets_octfile (what, depth); + targets = collect_targets_octfile (w, depth); elseif strcmp(type, 'texinfo') target = struct(); - target.name = what; + target.name = w; target.link = ''; target.depth = depth; - [target.docstring, target.error] = parse_texinfo(fileread(what)); + [target.docstring, target.error] = parse_texinfo(fileread(w)); targets = [target]; else target = struct(); - target.name = what; + target.name = w; target.link = ''; target.depth = depth; target.docstring = ''; @@ -242,31 +242,31 @@ -function target = collect_targets_function(what) +function target = collect_targets_function(w) target = struct(); - target.name = what; + target.name = w; if is_octave() target.link = ''; else - target.link = sprintf('%s', which(what), what); + target.link = sprintf('%s', which(w), w); end [target.docstring, target.error] = extract_docstring(target.name); end -function targets = collect_targets_class(what, depth) - if (strcmp(what(1), '@')) +function targets = collect_targets_class(w, depth) + if (strcmp(w(1), '@')) % Octave methods('@foo') gives java error, Matlab just says "No methods" - what = what(2:end); + w = w(2:end); end % TODO: workaround github.com/catch22/octave-doctest/issues/135 by % accessing all non-constructor method help text *before* "help obj" if (is_octave ()) - meths = methods (what); + meths = methods (w); for i=1:numel (meths) - if (~ strcmp (meths{i}, what)) % skip @obj/obj - name = sprintf ('@%s%s%s', what, filesep (), meths{i}); + if (~ strcmp (meths{i}, w)) % skip @obj/obj + name = sprintf ('@%s%s%s', w, filesep (), meths{i}); [docstring, format] = get_help_text (name); end end @@ -275,26 +275,26 @@ % First, "help class". For classdef, this differs from "help class.class" % (general class help vs constructor help). For old-style classes we will % probably end up testing the constructor twice but... meh. - target.name = what; + target.name = w; if is_octave() target.link = ''; else - target.link = sprintf('%s', which(what), what); + target.link = sprintf('%s', which(w), w); end target.depth = depth; [target.docstring, target.error] = extract_docstring(target.name); targets = target; % Next, add targets for all class methods - meths = methods(what); + meths = methods(w); for i=1:numel(meths) target = struct(); if is_octave() - target.name = sprintf('@%s%s%s', what, filesep(), meths{i}); + target.name = sprintf('@%s%s%s', w, filesep(), meths{i}); target.link = ''; else - target.name = sprintf('%s.%s', what, meths{i}); - target.link = sprintf('%s', which(what), meths{i}, target.name); + target.name = sprintf('%s.%s', w, meths{i}); + target.link = sprintf('%s', which(w), meths{i}, target.name); end target.depth = depth; [target.docstring, target.error] = extract_docstring(target.name); From 8f507c516cfa549e63a68e03db6280c877b1018d Mon Sep 17 00:00:00 2001 From: "Colin B. Macdonald" Date: Fri, 20 Sep 2019 12:06:42 -0700 Subject: [PATCH 3/4] tests: add a test that empty input does something reasonable Fixes #222. Well, it was already fixed somewhere since 0.7.0, so just add the test to make sure it doesn't regress. --- test/bist.m | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/bist.m b/test/bist.m index 8ba2dbe..d8a5015 100644 --- a/test/bist.m +++ b/test/bist.m @@ -128,3 +128,9 @@ function bist() %! [n, t, summ] = doctest ('bar'); %! assert (n == 1) %! assert (t == 2) + +%!test +%! [n, t, summ] = doctest({}); +%! assert (n == 0) +%! assert (t == 0) +%! assert (summ.num_targets == 0) From 330133a41fc507b3eaba0ab0e3a7f79cf4661aad Mon Sep 17 00:00:00 2001 From: "Colin B. Macdonald" Date: Fri, 20 Sep 2019 12:08:01 -0700 Subject: [PATCH 4/4] empty strings in target should be skipped --- inst/private/doctest_collect.m | 4 ++++ test/bist.m | 17 +++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/inst/private/doctest_collect.m b/inst/private/doctest_collect.m index 73d8575..c5f76ca 100644 --- a/inst/private/doctest_collect.m +++ b/inst/private/doctest_collect.m @@ -19,6 +19,10 @@ % TODO: methods('logical') octave/matlab differ: which behaviour do we want? % TODO: what about builtin "test" versus dir "test/"? Do we prefer dir? +if (isempty(w)) + return +end + % determine type of target if is_octave() % Note: ripe for refactoring once "exist(w, 'class')" works in Octave. diff --git a/test/bist.m b/test/bist.m index d8a5015..1dbe8fd 100644 --- a/test/bist.m +++ b/test/bist.m @@ -134,3 +134,20 @@ function bist() %! assert (n == 0) %! assert (t == 0) %! assert (summ.num_targets == 0) + +%!test +%! % skip empty targets +%! [n, t, summ] = doctest({'', ''}); +%! assert (n == 0) +%! assert (t == 0) +%! assert (summ.num_targets == 0) +%! assert (summ.num_targets_with_extraction_errors == 0) + +%!test +%! % skip empty targets +%! [n1, t1, summ1] = doctest('doctest'); +%! [n2, t2, summ2] = doctest({'', '', 'doctest', ''}); +%! assert (n1 == n2) +%! assert (t1 == t2) +%! assert (summ1.num_targets == summ2.num_targets) +%! assert (summ2.num_targets_with_extraction_errors == 0)