Run doctests as part of CI pipeline #9939
Conversation
| @@ -120,7 +120,10 @@ jobs: | |||
| - uses: Swatinem/rust-cache@v2 | |||
| - name: "Run tests" | |||
| shell: bash | |||
| run: cargo nextest run --workspace --status-level skip --failure-output immediate-final --no-fail-fast -j 12 | |||
There was a problem hiding this comment.
I removed the -j 12. It gives us minimal speedup but has the downside that it requires manual updating (that likely doesn't happen) when any properties about the runners change.
There was a problem hiding this comment.
Did you benchmark it? It seemed like it was about 30s faster for WIndows (#9921 (comment)).
There was a problem hiding this comment.
I did not but you mentioned on your PR that it is minimal 😆
Basically no difference:
There was a problem hiding this comment.
I ended up keeping it because the Windows difference was kinda big though...
MichaReiser
force-pushed
the
doctests-
branch
from
7904a1e
to
4e7f8f3
Compare
MichaReiser
force-pushed
the
doctests-
branch
from
4e7f8f3
to
ecb5815
Compare
| @@ -0,0 +1,8 @@ | |||
| [profile.ci] | |||
There was a problem hiding this comment.
cargo insta doesn't support passing custom test runner flags. Luckily, nextest supports setting the profile via environment flag..
MichaReiser
force-pushed
the
doctests-
branch
from
ecb5815
to
85ec424
Compare
|
| run: cargo nextest run --workspace --status-level skip --failure-output immediate-final --no-fail-fast -j 12 | ||
| env: | ||
| NEXTEST_PROFILE: "ci" | ||
| run: cargo insta test --all-features --unreferenced reject --test-runner nextest |
There was a problem hiding this comment.
Oh I didn't know about this, huh. Did you know about this @zanieb?
There was a problem hiding this comment.
Yeah I talked about this in my earlier pull request I linked :p I think the key here is setting the profile via environment variable? Nice work @MichaReiser
|
It looks like this PR also re-adds |
| run: cargo nextest run --workspace --status-level skip --failure-output immediate-final --no-fail-fast -j 12 | ||
| run: | | ||
| cargo nextest run --all-features --profile ci | ||
| cargo test --all-features --doc |
There was a problem hiding this comment.
Why is this on the Windows tests? If anything, shouldn't it only be on the Linux tests, since doctests add so much overhead, and Windows is the bottleneck?
There was a problem hiding this comment.
cargo insta uses nextest to run the "normal" test but calls cargo test --doc for doctests.
So the doctests run on both Windows and Linux.
However, we never used the unreferenced-snapshots feature on Windows, so it felt useless to use cargo insta on Windows for a feature that we don't need. That's why it makes the call directly.
There was a problem hiding this comment.
Ah that makes sense, thanks! Doctests are so slow, honestly I'm tempted not to run them on Windows.
Summary
Add explicit command to run doctest. Run tests for all features.
Fixes #9938
Test Plan
See CI pipeline