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

RFC: Stop testing feature combinations exhaustively #672

Closed
lnicola opened this issue Oct 14, 2021 · 2 comments · Fixed by #673
Closed

RFC: Stop testing feature combinations exhaustively #672

lnicola opened this issue Oct 14, 2021 · 2 comments · Fixed by #673

Comments

@lnicola
Copy link
Member

lnicola commented Oct 14, 2021

It looks like the tests take about 12 minutes or so, but the CI workflow uses cargo-all-features, which checks all combinations of features. We currently have three of them, which means that the tests (probably) run 8 times.

Rust features are supposed to be additive. I realize this isn't always the case (e.g. the PROJ network feature might influence the results), but there's not that much place for strange interactions between them:

  • use-proj only re-exports some proj types AFAICT
  • proj-network only enables a proj feature; this could presumably influence some (doc-)tests though
  • serde only enables the serde integration

So I think it would be relatively safe to only run cargo check --no-default-features and cargo test --all-features, and maybe trim down the tests where proj-network could have an impact.

@urschrei
Copy link
Member

I think that we probably don't have to test proj-network since any failure would occur and be evident upstream – we already test that enabling the network functionality changes the results in the proj crate, so barring a failure mode in which we accidentally deploy a proj crate version despite broken functionality and tests I think we can just use use-proj here?

@lnicola
Copy link
Member Author

lnicola commented Oct 14, 2021

I agree, testing with proj-network only introduces new failure modes for us: getting different results because of the extra grids and not working because of transient network issues.

@lnicola lnicola mentioned this issue Oct 16, 2021
2 tasks
@bors bors bot closed this as completed in a406c8d Oct 18, 2021
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 a pull request may close this issue.

2 participants