Feature idea: file-level filtering option

[nrser]

Summary

Add a CLI option to only doc-test source files containing a special comment or YARD tag.

This would allow for gradual file-by-file incorporation into existing code bases that already have @example tags all over the place that were not written to doc-test.

Like how Flow uses // @flow comments to activate type checking at a file level, if you happen to have used that. I swear I've seen this sort of thing in other softwares as well, but they're not coming to mind at the moment.

Usage

Add comments or tags to the desired file and add a flag to the CLI command, something like:

yard doctest --opt-in

I'm not sure about the name --opt-in, but I'm having trouble thinking of something better right now.

Background

I'm doing this right now by sticking a

# doctest: true

comment in the files that I've migrated over to use doc-tests, then running via a harness script

bundle exec \
  yard doctest $@ \
    $(rg --files-with-matches '# doctest: true' --glob='*.rb' ./lib/)

However, I think it would ease adoption into existing code bases to have this functionality built-in. And I'd like to see more adoption, 'cause I feel there is little more frustrating than wasting precious time with new software in "what the hell am I doing wrong?"-land only to eventually figure out that the example itself is broken.

Implementation Thoughts

Ruby already has "magic" file-level comments like # frozen_string_literal: true, etc., so there's some basis for the # doctest: true approach, though it might be confusing since the other magic comments people are used to seeing are Ruby VM directives, and this is not. But magic comments are also used to tell editors things about the file and such, so it seems reasonable. Maybe # yard-doctest: true would be more clear.

Using a YARD tag is possibly another option, like @doctest true, but it's a least a bit more complicated... I'm not familiar how (if at all) YARD binds file-level doc-strings as I've only ever used "code-object"-level stuff.

A YARD tag could be naturally be extended to finer granularity: putting @doctest true or @doctest false on method docs to switch those on or off individually.

[p0deje]

Thank you for such a detailed proposal, I appreciate that!

In the current implementation, we can filter the files to use using Rake task's pattern:

require 'yard/doctest/rake'

YARD::Doctest::RakeTask.new do |task|
  task.pattern = 'app/**/*.rb'
end

Your point of more adoption is a valid one, so it makes sense to allow for one-by-one adoption. In this case, we need @doctest [true|false] tag which will make the examples as tests. I am not sure if it should be turned on or off by default though, so your feedback is appreciated.

Another possible solution is to generate configuration which will skip all failing tests by default. Something similar to RuboCop --auto-gen-config. The result of such a command can be doctest_helper_example.rb with a bunch of doctest#skip calls.

[p0deje]

In regards to # doctest: true I'd like to avoid confusing people this is something similar to Ruby's pragamas (encoding, frozen_string_literal).

[nrser]

TL;DR I like the config generation idea. It doesn't complicate the runtime code, it gives you a clear list of what doesn't work, doesn't involve any file-flagging mess, and lets you get going right away in an existing codebase.

On Implementation CLI or Rake task? Does pretty much everyone depend on Rake (and if you don't, well, do if you want to use this)?

I personally never liked Rake much... the "ENV vars are kind-of arguments sometimes and oh let's also use short generic ones like version that you might have defined for any old reason especially in containers" thing has burnt me bad, but I acknowledge I'm probably in a sever minority here. And I do depend on it everywhere.

---

The way I understand the config generation approach is that you would want to generate the config when you first added YARD Doctest, since that's the only time a failed example may not mean a failed doctest, then probably update it by hand any time you added an example that isn't meant to doctest?

Yeah, that seems like it would work. It gives you a list of everything being skipped in one place, which is nice if you're aiming to eventually aiming to have all @example doctest. It definitely hits the "I just added doctest and have a lot of examples that won't work, but should, and I want to migrate them gradually", which is the use case I've been talking about.

The major downside I see is having to remember check that list to make sure I don't think something is working when it's getting skipped, and possibly end up incorporating a false pretense into the library if you forget or miss it.

I realize that in addition to Ruby that "should run", I traditionally use @example for pseudo-esque code that will never run, as the tag just seemed like the place to put that stuff, but this - as well as skipping and file-flagging - lead to a weird situation for the user reading the docs: there's no way to know which examples are tested and which aren't. You may end up back at spending forever trying to get something to run that was never intended to, which is exactly what I don't want for users.

It seems like doctest's inherent paradigm is all-or-nothing: you doctest, and all @example should be valid doctests, or you don't. This has the major advantage of being simple.

From this angle, I like the config generation solution: it doesn't mess with the runtime code but it can buy you some time to get everything in order, with a clear list of what isn't yet.

The other road - some @example are doctest and some just are not - seems fought with peril.
You would want a way of clearly marking in source and generated docs what is a doctest and what isn't, and that's just a terrible amount of complication for something that is nice and simple now.

There's always code blocks to demonstrate stuff that doesn't run.

[p0deje]

The more I think about this, the more it seems to me that yard-doctest can behave in the following way:

1. On first installation, a user runs yard doctest --auto-gen-config which creates or appends its configuration to doctest_helper.rb
2. The command collects all examples and runs them, collecting those which are failing.
3. The commands creates a configuration which marks all failing tests as "pending" and dumps the configuration to file. We can use doctest#pending for such examples.
4. The next time user runs yard doctest, it runs all examples, but ignores the ones that are marked as "pending" inside configuration file. If example is passing now (e.g. user implemented it correctly) but it's marked as "pending" - command will say that this test should be removed from the configuration file. The logic is similar to RSpecs skip vs pending.

This way we can get initially passing doctests and disable those that are not passing, but also guarantee that as soon as any they start passing - they won't be forgotten.

[p0deje]

The other road - some @example are doctest and some just are not - seems fought with peril.
You would want a way of clearly marking in source and generated docs what is a doctest and what isn't, and that's just a terrible amount of complication for something that is nice and simple now.

My idea was that any @example can be used by a user as a "working" example. Can you share some of the examples which should not be "working" in a sense they should not be doctests?

[chriscz]

@p0deje One case that comes to mind is if there is a significant amount of context required for the example to work. As a very simple example

@example
CancellationRequest.new(booking: some_booking)

However one could argue that in such a scenario you should have some static pre-amble module to get your context. Would possibly become (or event just factory)

@example
CancellationRequest.new(booking: DocTestContext.factory(:booking, :confirmed))

Disabling doctest on a per-example level instead of a per-file level makes most sense to me.

[p0deje]

@chriscz In this particular scenario, why don't you want to provide some_booking via doctest_helper.rb and keep running the test for the example?

[chriscz]

That's my mistake @p0deje, I see it in the README. Thank you!