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

Add CI testing for all feature flag combinations and fix all reported errors #475

Merged
merged 5 commits into from
Feb 20, 2024

Conversation

Shute052
Copy link
Collaborator

@Shute052 Shute052 commented Feb 20, 2024

Objective

Make sure we thoroughly test all feature-related code and updates.

Solution

Cover all combinations of features our crate offers when running CI tests for clippy, doc-check, doc-test, test, and compile-check.

For instance, given the current existence of four features: asset, ui, block_ui_interactions, and egui, the new CI testing will include scenarios like:

  • Testing with --no-default-features
  • Testing with --all-features
  • Testing with individual features enabled:
    • --features=asset
    • --features=ui
    • --features=block_ui_interactions
    • --features=egui
  • Testing with various combinations of features enabled:
    • --features=asset,ui
    • --features=asset,block_ui_interactions
    • --features=asset,egui
    • --features=ui,block_ui_interactions
    • --features=ui,egui
    • --features=block_ui_interactions,egui
    • --features=asset,ui,block_ui_interactions
    • --features=asset,ui,egui
    • --features=asset,block_ui_interactions,egui
    • --features=ui,block_ui_interactions,egui

This ensures we catch any regressions or issues related to the features.

TODO

  • fix all reported errors (without those reporting from doc-check)

Changelog

  • Add new CI testing for all feature flag combinations

@Shute052 Shute052 changed the title Add CI testing for every potential combination of crate feature flags Add CI testing for all feature flag combinations and fix all reported errors Feb 20, 2024
@Shute052
Copy link
Collaborator Author

Shute052 commented Feb 20, 2024

When running doc-check with scenarios excluding the ui feature, the new testing reports the following issues:

error: unresolved link to `crate::systems::update_action_state_from_interaction`
  --> src/action_driver.rs:20:58
   |
20 | /// By default, [`update_action_state_from_interaction`](crate::systems::update_action_state_from_interaction) uses this component
   |                                                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no item named `update_action_state_from_interaction` in module `systems`
   |
   = note: `-D rustdoc::broken-intra-doc-links` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(rustdoc::broken_intra_doc_links)]`

error: unresolved link to `crate::systems::update_action_state_from_interaction`
  --> src/action_driver.rs:55:71
   |
55 | ...n`](crate::systems::update_action_state_from_interaction) for an example of how this is done.
   |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no item named `update_action_state_from_interaction` in module `systems`

error: unresolved link to `Interaction`
   --> src/input_mocking.rs:[119](https://github.com/Leafwing-Studios/leafwing-input-manager/actions/runs/7970380465/job/21757867601?pr=475#step:6:120):26
    |
119 |     /// as well as any [`Interaction`] components and all input [`Events`].
    |                          ^^^^^^^^^^^ no item named `Interaction` in scope
    |
    = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`

error: unresolved link to `crate::systems::update_action_state_from_interaction`
  --> src/plugin.rs:52:48
   |
52 | /// - [`update_action_state_from_interaction`](crate::systems::update_action_state_from_interaction), for triggering actions from buttons
   |                                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no item named `update_action_state_from_interaction` in module `systems`

error: unresolved link to `bevy::ui::Interaction`
  --> src/plugin.rs:53:[122](https://github.com/Leafwing-Studios/leafwing-input-manager/actions/runs/7970380465/job/21757867601?pr=475#step:6:123)
   |
53 | ...component based on an [`Interaction`](bevy::ui::Interaction) component
   |                                          ^^^^^^^^^^^^^^^^^^^^^ no item named `ui` in module `bevy`

@Shute052
Copy link
Collaborator Author

Shute052 commented Feb 20, 2024

All these unresolved links occur because they are excluded when the ui feature is disabled.
Alternatively, we could only run doc-check with the --all-features option to ignore those errors.

@Shute052 Shute052 added the build system Automatic testing label Feb 20, 2024
@alice-i-cecile
Copy link
Contributor

Let's run doc check only with all features. I care much more about the links working on docs.rs than locally.

@Shute052
Copy link
Collaborator Author

Shute052 commented Feb 20, 2024

In that case, this PR shouldn't have any further actions needed, at least for now.

@Shute052
Copy link
Collaborator Author

Shute052 commented Feb 20, 2024

To validate everything's working as expected, try removing some of the code that depends on feature flags. This should trigger the errors we're looking for.

For example, parts like this mentioned

/// Remove this cfg in the 'src/input_map.rs'
#[cfg(feature = "asset")]
use bevy::asset::Asset;

@alice-i-cecile alice-i-cecile merged commit 9628f6b into Leafwing-Studios:main Feb 20, 2024
4 checks passed
@Shute052 Shute052 deleted the ci-test branch February 20, 2024 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build system Automatic testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants