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

test: run doctests in just test, fix broken doctests #282

Merged
merged 4 commits into from
Sep 17, 2023
Merged

Conversation

hawkw
Copy link
Contributor

@hawkw hawkw commented Sep 17, 2023

Because we're using cargo nextest, which can't easily learn about
doctests from RustDoc, the just test command currently only runs
normal unit and integration tests. This means that we have a bunch of
doctests which were silently not being run on CI or when running tests
locally through the Justfile. Whoopsie!

This branch adds a cargo test --doc invocation to just test as well,
so that doctests are also run. Since CI also invokes just test, we
don't need to do anything else to ensure that doctests also happen on
CI.

After fixing this, I found that a bunch of our doctests are currently
broken, because...we were never actually testing them lol. In
particular:

  • The mnemos-abi crate's bbqueue_ipc module had a bunch of super
    broken tests referincing APIs that don't actually exist in this
    module. I believe this probably happened due to @jamesmunns copying
    code from his upstream bbqueue crate, changing the APIs, and
    forgetting to update the doctests? Anyway, I just removed them.
  • mnemos-config had a bunch of doctests which didn't compile any
    longer because I had renamed some APIs and didn't update them, so I
    fixed that.

Because we're using `cargo nextest`, which can't easily learn about
doctests, the `just test` command currently only runs normal unit and
integration tests. This means that we have a bunch of doctests which
were silently not being run. Whoopsie!

This commit adds a `cargo test --doc` invocation to `just test` as well,
so that doctests are also run.
These doctests reference APIs that don't exist in this module. I
believe this probably happened due to @jamesmunns copying code from his
upstream `bbqueue` crate, changing the APIs, and forgetting to update
the doctests? Anyway, I just removed them.
@hawkw hawkw enabled auto-merge (squash) September 17, 2023 17:33
@hawkw hawkw merged commit 24e6939 into main Sep 17, 2023
10 checks passed
@hawkw hawkw deleted the eliza/fix-doctests branch September 17, 2023 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant