-
Notifications
You must be signed in to change notification settings - Fork 190
Enable cargo-nextest
for CI
#860
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
Conversation
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
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.
nice,
it still doesn't support doc tests right?
might be good to add another job for that..
No, still not nextest-rs/nextest#16 I can suggest editing the |
Signed-off-by: Alexandru Vasile <[email protected]>
.gitlab-ci.yml
Outdated
@@ -103,7 +103,9 @@ test: | |||
<<: *docker-env | |||
<<: *common-refs | |||
script: | |||
- cargo test | |||
- cargo install cargo-nextest --locked |
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 is not necessary, the image already contains cargo-nextest
(not the latest version though)
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.
Thanks! I'll remove it, does cargo-hack
also exist on the image?
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.
No, we can add it if needed. But it might take some time when it reaches the production image
Signed-off-by: Alexandru Vasile <[email protected]>
So to confirm, it looks like (Does that include the additional That being the case, I'd prefer to keep it simple and stick with |
wow, I thought nextest was suppose to faster... but I think the UI/proc macro tests are the bottleneck |
Is the verdict to close this PR since nexttest is actually slower anyway (plus doesn't run doctests)? |
To avoid issues with the CI, the nextest was replaced with
cargo test
.This PR ensures we utilize it again the
cargo nextest
functionality, viainstalling it locked (commit)
Issue reference: nextest-rs/nextest#306
Time Delta
Sample 1: 6fdb67f
Sample 2: bd31557.