Skip to content

chore(ci): use --feature-powerset --depth 2 in features check #2423

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

Merged
merged 2 commits into from
Feb 6, 2021

Conversation

taiki-e
Copy link
Contributor

@taiki-e taiki-e commented Feb 6, 2021

This allows catching the issue like #2421.

also our CI didn't notice the failure of combining --features http1,client,server.

This can be fixed by using cargo hack --feature-powerset (all combination of features) instead of cargo hack --each-feature (each feature) in CI.
However, if we use --feature-powerset, there will be more than 700 feature combinations, so we may need to use --depth (specify a max number of simultaneous feature flags of --feature-powerset) flag to limit the number of combinations.

We have too many features to check the full powerset (776), so limit the max number of simultaneous feature flags to 2 by --depth option. I think this should be sufficient in most cases as carllerche said in taiki-e/cargo-hack#58.

Tokio has too many features to test the power set. However, most issues can be reproduced by testing at most 2 features simultaneously.

In fact, #2421 can be reproduced with a combination of two feature flags. (--features client,http1)

@taiki-e taiki-e mentioned this pull request Feb 6, 2021
@taiki-e
Copy link
Contributor Author

taiki-e commented Feb 6, 2021

CI failure is unrelated and will be fixed in #2424.

@seanmonstar
Copy link
Member

This seems right. There may be a couple extra combinations that are interesting, but perhaps they should just be explicitly added in CI instead of blowing up the depth.

@seanmonstar seanmonstar merged commit dfa1bb2 into hyperium:master Feb 6, 2021
@taiki-e taiki-e deleted the feature-powerset branch February 6, 2021 15:37
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 this pull request may close these issues.

2 participants