Gut icon indicating copy to clipboard operation
Gut copied to clipboard

Allow inner class tests without requiring the top-level script to be a test

Open strank opened this issue 4 years ago • 5 comments

This modifies test collection to only add scripts that actually inherit from GutTest, but still look for inner classes that are tests in any case. This way adding tests in inner classes to any .gd file is possible. That's great for adding small documentation-like tests without having to create separate test files.

strank avatar Feb 18 '22 02:02 strank

I'm not sure I like having tests and production code mingled. Do you really want to ship code with tests in it? If this is a convention in other languages then I'll consider it.

bitwes avatar Apr 29 '22 00:04 bitwes

What about Rust? https://doc.rust-lang.org/rustdoc/documentation-tests.html

3ter avatar Apr 30 '22 06:04 3ter

What kind of debauchery is Rust documentation-tests? What is going on with Rust comments in general? There's like 20 different ways to comment things.

"Sorry, I can't do anything tonight, I'm debugging my comments. There's something wrong with:

// let s = "foo
// # bar # baz";

I don't know if I need // or /// then # or ## or # and then ##. Maybe I need a /* or /** or /*** or //! or /*! or /*!!. I'm just trying to show a line in my executable markdown comment block that also supports hiding executable lines (using a comment indicator from another language) from the generated documentation. Also I had to start over because someone added a comment that deleted all the code when I tried to test my runnable comments."

Jokes aside, I can see some benefits of this. I wonder if having another type of test script (GutDocTest?) to inherit from would be a feasible approach. You could then run GUT with a flag to run documentation tests and it would look at every script and find GutDocTest inner classes and run them. This would prevent a run from mixing the two together since it doesn't seem like you would want to do that. We could also add a flag to run both. GutDocTest could then be intermingled with other files or be in their own standalone file. Maybe later GutDocTest could get additional features for generating documentation.

I'm open to more discussion. Just don't ask to add executable comments.

bitwes avatar Apr 30 '22 17:04 bitwes

I prefer real documentation tests where tests are directly in comments - so no qualms about publishing "unused" code there. So if we had those, that would be perfect

Having said that: there's no real reason to require inner class tests to only be in a file that happens to also derive from the test class! Not needed for discovery. No other technical reason. In fact, inner classes in gdscript have almost no connection with the outer class except that they are defined in the same file. That alone would be reason enough for me to remove the requirement. The main benefit I see is the flexibility to use it if you want it. Best use case is probably nice self contained demonstrations. (My use case coincidentally ;) )

Btw, rust comment syntax is actually quite reasonable considering that they want to allow block comments and line comments and distinguish between inner and outer comments. You might argue that they shouldn't have that many...

strank avatar May 02 '22 17:05 strank

Documentation tests is not a feature I would implement in GUT. I would entertain PRs for it. As far as I know GDScript doesn't even have a javadoc equivalent yet and documentation tests feels more like a feature for a documentation generation tool than for the testing tool. Something that is already parsing comments could leverage GUT to run tests, but I don't think it should be the other way around.

As for allowing inner test classes anywhere, there are reasons that restriction exists. Originally GUT parsed scripts to find all the inner classes and tests In order to cut down on the number of files that were parsed, the containing script is first checked to see if it is a test script. GUT now uses metadata about a script provided by Godot, leaving the parsing up to Godot. There still might be some overhead if GUT has to check every inner class in every script to find all tests. This would have to be tested.

Another reason for the top level script requirement is for readability/maintainability. One could argue that the test script prefix should be enough to indicate a test script but that's not always the case (people name things test_ and do things like configure res:// directory and "include all subdirectories"). Having the base script be a GutTest makes it clear that the entire script is a test script to both the user and GUT. When you are the only "user" in a project this isn't a big deal, but it helps with maintainability of larger projects with multiple programmers.

To fully consider this PR I would also need a flag to enable/disable this feature. It should be disabled by default. The flag should be settable from the command line and the GutPanel (I could help here as it is pretty gross to make changes to).

If you can verify there is no performance issues, add tests to verify things work correctly with and without the flag enabled, and that this doesn't break anything in GUT then I would add this in.

bitwes avatar May 03 '22 16:05 bitwes

Repoen if you start working on this again.

bitwes avatar Sep 21 '22 16:09 bitwes