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

Add basic support for doctests #9315

Merged
merged 6 commits into from
Jan 30, 2025

Conversation

garazdawi
Copy link
Contributor

@garazdawi garazdawi commented Jan 20, 2025

This PR adds basic support for running doctests on documentation in Erlang/OTP. It is not yet made available for public use as the API may change, but we it can be used within Erlang/OTP to test documentation examples.

The syntax for writing doctests is like this:

```
> 1 + 2.
3
```

When something like the above is found in the docs, the expression will be run and tested against the return value. It is possible to write more complex things, but that it is the basic idea.

@garazdawi garazdawi added team:VM Assigned to OTP team VM enhancement testing currently being tested, tag is used by OTP internal CI labels Jan 20, 2025
@garazdawi garazdawi added this to the OTP-28.0 milestone Jan 20, 2025
@garazdawi garazdawi self-assigned this Jan 20, 2025
Copy link
Contributor

github-actions bot commented Jan 20, 2025

CT Test Results

    4 files    226 suites   1h 54m 43s ⏱️
3 647 tests 3 554 ✅  93 💤 0 ❌
4 741 runs  4 625 ✅ 116 💤 0 ❌

Results for commit 1e6b11f.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

// Erlang/OTP Github Action Bot

@garazdawi garazdawi mentioned this pull request Jan 20, 2025
lib/stdlib/src/shell_docs_test.erl Show resolved Hide resolved
lib/stdlib/src/shell_docs.erl Outdated Show resolved Hide resolved
lib/stdlib/src/shell_docs_test.erl Show resolved Hide resolved
lib/stdlib/src/shell_docs_test.erl Show resolved Hide resolved
lib/stdlib/src/shell_docs_test.erl Show resolved Hide resolved
lib/stdlib/src/shell_docs_test.erl Outdated Show resolved Hide resolved
lib/stdlib/src/shell_docs_test.erl Show resolved Hide resolved
lib/stdlib/src/shell_docs_test.erl Show resolved Hide resolved
lib/stdlib/src/shell_docs_test.erl Outdated Show resolved Hide resolved
lib/stdlib/src/shell_docs_test.erl Outdated Show resolved Hide resolved
Copy link
Contributor

@u3s u3s left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting. looks useful, i wonder how/if it could be extended for code samples requiring some env preparation setup ...

lib/stdlib/test/lists_SUITE.erl Outdated Show resolved Hide resolved
@garazdawi
Copy link
Contributor Author

i wonder how/if it could be extended for code samples requiring some env preparation setup ...

In the zstd_SUITE tests I needed this so there is support for passing an erl_eval:binding_struct/0 to shell_docs:test/2 which will allow you to pass variables to specific tests.

The example tested using this is zstd:get_dict_id/1.

lib/stdlib/src/lists.erl Outdated Show resolved Hide resolved
@josevalim
Copy link
Contributor

@garazdawi this is very exciting! I understand this is initial approach but here are some questions to consider:

  1. Do you handle multiline outputs?

  2. How do you separate distinct doctests? In Elixir, we consider any empty lines after the doctest output to be a new test. This is particularly important for Erlang, because you can't reassign variables. In other words, which of these should be considered two doctests?

    > List = [1, 2, 3],
    > lists:sum(List).
    6

    > List = [1, 2, 3, 4],
    > lists:sum(List).
    10
```erlang
> List = [1, 2, 3],
> lists:sum(List).
6

> List = [1, 2, 3, 4],
> lists:sum(List).
10
```
    > List = [1, 2, 3],
    > lists:sum(List).
    6
    > List = [1, 2, 3, 4],
    > lists:sum(List).
    10
```erlang
> List = [1, 2, 3],
> lists:sum(List).
6
> List = [1, 2, 3, 4],
> lists:sum(List).
10
```

Elixir considers the first two to have two separate doctests. FWIW, the reason why we detect empty lines is precisely because we encourage the first style, where you don't have to wrap your code in ```elixir and, in those cases, the markdown engine considers the first example to be a single <code> chunk. However, you can take the opposite route, and ask people to explicitly tag everything with ```erlang.

@garazdawi
Copy link
Contributor Author

  1. Yes.
  2. Not yet as all the examples I have come across so far have been self-contained.

@garazdawi
Copy link
Contributor Author

I pushed a bunch of examples showing what can be done. Copied below to be easier to find:

## Basic example:

```
> 1+2.
3
```

## Basic example using erlang code:

```erlang
> 1+2.
3
```

## Multi-line prompt example:

```erlang
> 1
  +
  2
  .
3
```

## Multi-match example:

```erlang
> [1, 2].
[
 1
 ,
 2
 ]
```

## Multiple prompts:

```
> 1 + 2.
3
> 3 + 4.
7
```

## Ignore result:

```
> 1 + 2.
```

## Defining variables:

```
> A = 1+2.
> A + 3.
6
```

## Comments:

```
> [1, 
% A comment in between prompts
  2].
[1,
% A comment in a match
 2]
> [1, 
  % Indented comment in between prompts
  2].
[1,
 % Indented comment in a match
 2]
```

## Prebound variables:

```
> Prebound.
hello
```

Extra features that I'm thinking of adding are:

  • Defining a function just as we can do in the shell
  • Testing crashes by matching on exceptions

@williamthome
Copy link
Contributor

williamthome commented Jan 22, 2025

2. How do you separate distinct doctests? In Elixir, we consider any empty lines after the doctest output to be a new test. This is particularly important for Erlang, because you can't reassign variables. In other words, which of these should be considered two doctests?

IMHO multiple tests should be multiple code blocks, like:

> List = [1, 2, 3],
> lists:sum(List).
6
> List = [1, 2, 3, 4],
> lists:sum(List).
10

Or

> List1 = [1, 2, 3],
> lists:sum(List1).
6
> List2 = [1, 2, 3, 4],
> lists:sum(List2).
10

And not

> List = [1, 2, 3],
> lists:sum(List).
6

> List = [1, 2, 3, 4],
> lists:sum(List).
10

@garazdawi
Copy link
Contributor Author

I think José was suggesting:

> List = [1, 2, 3],
> lists:sum(List).
6

> List = [1, 2, 3, 4],
> lists:sum(List).
10

i.e. that there needs to be a double-newline to trigger a new test environment. I agree with you though that it probably is not needed.

@michalmuskala
Copy link
Contributor

michalmuskala commented Jan 22, 2025

@garazdawi Would it make sense to expose some sort of API that would return the tests as a data structure? Some sort of [{AST, ExpectedValue}] thing.

The motivation is that the errors you'd get from using = for asserting are kind of poor. There are ways to present it better - starting with just the stdlib ?assert macro, but also even more advanced similar to Elixir's rich assert diffs with syntax highlighting - we have a prototype for this for Erlang we're working to open-source.
I think having some sort of data-based API for this would make it easiest to integrate with those other assertion frameworks - by them wrapping it in their APIs and exposing something similar to shell_docs:test/2 API.

cc @robertoaloi

@josevalim
Copy link
Contributor

Hrm, relying on = for the assertions can also be very tricky for opaque data structures, no? If you use =:=, then I guess you could at least test sets as:

> sets:add_element(foo, sets:new()).
sets:from_list([foo])

But using pattern matching is probably impossible without showing the representation everywhere in the docs?

Although another way would be to write all of them as:

> Set = sets:add_element(foo, sets:new()),
> sets:is_element(foo, Set).
true

@garazdawi
Copy link
Contributor Author

@michalmuskala we can do that, but later on. I will keep things simple for now.

@josevalim alternative 2 is how I did it in the zstd module for opaques (#9316).

@garazdawi garazdawi force-pushed the lukas/stdlib/doctests branch from ca3b1ab to 160684e Compare January 23, 2025 10:39
@robertoaloi
Copy link
Contributor

i wonder how/if it could be extended for code samples requiring some env preparation setup ...

In the zstd_SUITE tests I needed this so there is support for passing an erl_eval:binding_struct/0 to shell_docs:test/2 which will allow you to pass variables to specific tests.

The example tested using this is zstd:get_dict_id/1.

In Rust, there's special syntax to allow setup code or hiding portions of the example. See https://doc.rust-lang.org/rustdoc/write-documentation/documentation-tests.html#hiding-portions-of-the-example

@garazdawi garazdawi force-pushed the lukas/stdlib/doctests branch from b062ab6 to 1c14565 Compare January 28, 2025 09:25
@garazdawi garazdawi force-pushed the lukas/stdlib/doctests branch from 1e6b11f to edc69ed Compare January 30, 2025 09:20
@garazdawi garazdawi merged commit d628424 into erlang:master Jan 30, 2025
9 checks passed
@garazdawi
Copy link
Contributor Author

Merged! Looking forward to everyone helping out writing doctests for all our modules now! 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement team:VM Assigned to OTP team VM testing currently being tested, tag is used by OTP internal CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants