fix: lazy doc test and add unstable feature tests to CI #418
Conversation
| @@ -27,4 +27,4 @@ jobs: | |||
| - name: Add wasm32 target | |||
| run: rustup target add wasm32-unknown-unknown | |||
| - name: Test | |||
| run: cargo test --all | |||
| run: cargo test --all --features unstable | |||
There was a problem hiding this comment.
Let's add a separate step for this or add an &&, to make sure we don't break code without the feature.
And, while we are at it, s/all/workspace, --all is deprecated.
There was a problem hiding this comment.
Yeah, so the reason I didn't was that the unstable feature is purely additive and the CI is already very time-consuming. @mikedotexe have any thoughts here?
There was a problem hiding this comment.
Yeah, so the reason I didn't was that the unstable feature is purely additive
This is exactly the thing which is good to verify on CI. But I agree that this is not critical.
CI is already very time-consuming.
Oh wow, 20min for CI for the project of this scale does seem outrageous. rust-analyzer's CI takes 7ish minutes (example), so there's probably a room for improvement here
There was a problem hiding this comment.
Yeah my gut is to have a separate step as well, although CI that would add even more time. I know with Travis we could add automated weekly/monthly checks, perhaps we could do this and have our team get alerts. Thoughts, @austinabell ?
There was a problem hiding this comment.
Yeah my gut is to have a separate step as well, although CI that would add even more time. I know with Travis we could add automated weekly/monthly checks, perhaps we could do this and have our team get alerts. Thoughts, @austinabell ?
Can do intervals with actions as well, although this seems like a stop-gap solution I'm happy to add that check if you guys agree that's a good temporary solution.
|
Can we pull this in for now and improve the CI later? tests are currently failing with this feature enabled |
|
Yeah, it always makes sense to prioritize CI improvements over CI cleanups! |
I noticed just after #409 was pulled in that the doc test was not updated for the change from
collectionstostore, sorry!Wasn't caught because the
unstablefeature flag was not updated to be enabled on the CI, so I added that as well.