-
Notifications
You must be signed in to change notification settings - Fork 11
Add doctests (as nextest doesn't do these yet) #101
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 doctests (as nextest doesn't do these yet) #101
Conversation
.github/workflows/check.yaml
Outdated
- name: Run doc tests | ||
run: | | ||
cargo test --doc --all-features |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we create JSON output from nextest, maybe we also want to create JSON output here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree - forgot to take over that bit.
.github/workflows/check.yaml
Outdated
- name: Run doc tests | ||
run: | | ||
cargo test --doc --all-features > doctestresults--all-features.json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will not produce JSON output
for that, you need to provide some command line params:
RUSTC_BOOTSTRAP=1 cargo test --doc -- -Z unstable-options --format json --report-time > doctestresults--all-features.json
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really shouldn't be doing this while being at a conference...
.github/workflows/check.yaml
Outdated
name: doctest-results | ||
path: doctestresults--all-features.json | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing EOL
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
As has come up recently, our check workflow doesn't yet run doctests. So this is adding an explicit doctest step.
(refer to last bullet in #84)