Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature request: runtests @class #12

Closed
cbm755 opened this issue Mar 6, 2019 · 14 comments
Closed

feature request: runtests @class #12

cbm755 opened this issue Mar 6, 2019 · 14 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@cbm755
Copy link
Contributor

cbm755 commented Mar 6, 2019

I'd like to see runtests be the main driver. So runtests @myclass would run the tests in class @myclass.

Consider looking at Doctest:
https://github.com/catch22/octave-doctest/blob/master/inst/doctest.m
There we wanted doctest thing to DTRT:

  • doctest my_fcn works
  • doctest @klass works
  • doctest octfile.oct works (and is subtly different than doctest octfile.
  • doctest dir works recursively, possibly staying out of private/ subdirs (I forgot).
@mtmiller
Copy link
Contributor

mtmiller commented Mar 6, 2019

Why not test @myclass?

@cbm755
Copy link
Contributor Author

cbm755 commented Mar 10, 2019

A few years ago, I posted some patches to do some of this:

https://savannah.gnu.org/patch/?9220

Feel free to cherry pick if these still apply.

@mtmiller
Copy link
Contributor

Is there a reason why @-sign would be required or preferable for this? Or would test myclass be sufficient?

@cbm755
Copy link
Contributor Author

cbm755 commented Mar 12, 2019

Good question... doctest seems to usually do the same thing for doctest @myclass and doctest myclass..., although I checked the code and their are some subtleties, e.g., doctest @logical (Symbolic has @logical/isAlways) methods and doctest logical (the builtin)

>> pkg load symbolic
>> pkg load doctest
>> doctest logical
Doctest v0.6.1: this is Free Software without warranty, see source.

logical ................................................ NO TESTS

Summary:

   PASS    0/0   

1/1 targets passed, 1 without tests.

>> doctest @logical
Doctest v0.6.1: this is Free Software without warranty, see source.

logical ................................................ NO TESTS
@logical/isAlways ...................................... PASS    2/2   

Summary:

   PASS    2/2   

2/2 targets passed, 1 without tests.

I think the same distinction could apply for #! BISTs.

@apjanke
Copy link
Owner

apjanke commented Mar 17, 2019

Is there a reason why @-sign would be required or preferable for this? Or would test myclass be sufficient?

You might need it to disambiguate between running tests for the whole class, and running tests for just the constructor. If you have a class named foo, then the function foo is the constructor for it, and matches the test <function_name> calling form, too.

@apjanke apjanke self-assigned this Mar 17, 2019
@apjanke apjanke added the enhancement New feature or request label Mar 17, 2019
@mtmiller
Copy link
Contributor

You might need it to disambiguate between running tests for the whole class, and running tests for just the constructor.

Yeah, there was a similar discussion over at gnu-octave/octave-doctest#196, you might be interested in weighing in over there too.

@cbm755
Copy link
Contributor Author

cbm755 commented Mar 17, 2019

I posted a summary of my thinking based on that thread here

I think the same logic would apply for regular tests. Which suggests there could be some "parse this object for tests" common code.

@apjanke
Copy link
Owner

apjanke commented Mar 17, 2019

I think I mostly agree, and think that the whole test selection logic (and command syntax) for doctest and this BIST stuff can probably be the same.

My current thinking is that there are two layers of test discovery:

  1. The lower layer that operates on files, using files to identify tests, and a parser that reads a source file and extracts N tests from it.
  2. A higher user-facing layer that takes object names, directories, or other specifiers, and maps them to sets of files to draw tests from.
  3. A command syntax layer on top of that that interprets the string arguments passed in to the test or runtests command-style functions and parses them into the specifier data structures used by layer 2.

But then if you wanted to test just a single method or constructor from a class, you couldn't use the specifier -> files -> tests mapping, because you'd only want some selected tests from within a classdef file. So that's probably not the right design after all.

@apjanke
Copy link
Owner

apjanke commented Mar 17, 2019

Oh – can BISTs actually be associated with a particular method within a classdef file? All the ones I recall seeing just have all the tests in a block down at the bottom of the file.

@cbm755
Copy link
Contributor Author

cbm755 commented Mar 17, 2019

I think almost everything about Octave's classdef support is in flux. What is certainly true is that separate .m files specifying class methods can have their own tests associated with them. That is @klasdef/mymethod.m as opposed to methods defined "inline" in @klasdef/klasdef.m.

@apjanke
Copy link
Owner

apjanke commented Mar 17, 2019

Ah, that makes sense. And that way, the 1 specifier -> 1 or more files -> all tests from those files mapping sequence works.

@apjanke
Copy link
Owner

apjanke commented Mar 19, 2019

This is in. It's in runtests2_refactor. You can call it with either of these forms:

runtests2_refactor @myclass
runtests2_refactor -class myclass

You can also do runtests2_refactor myclass, and it might work, depending on what else "myclass" could resolve to.

Support for namespaces and dot-referencing of constructors or class methods is not supported yet.

Why not test @myclass?

As things stand now, test acts on a single file, and classes may have multiple files in their definition. Unless we merge runtests and test, then the support needs to go at the runtests level.

@apjanke
Copy link
Owner

apjanke commented Mar 19, 2019

Closing this as fixed since the basic mechanism is working. Adding a #49 for adding namespace support.

@apjanke apjanke closed this as completed Mar 19, 2019
@apjanke apjanke added this to the 0.4.0 milestone Mar 19, 2019
@apjanke
Copy link
Owner

apjanke commented Mar 19, 2019

Ha! I found a quick-and-dirty way to add namespace support. 3990564

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants