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

Check all option combinations in CI #26

Merged
merged 6 commits into from
Oct 31, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,14 @@ on:
pull_request:
paths-ignore:
- "**/README.md"
schedule:
- cron: "50 7 * * *"
workflow_dispatch:
inputs:
all_combinations:
description: 'Checks all combinations of options'
required: true
type: boolean

env:
CARGO_TERM_COLOR: always
Expand Down Expand Up @@ -53,8 +60,14 @@ jobs:
- uses: Swatinem/rust-cache@v2

- name: Generate and check project
if: github.event_name != 'schedule' && !inputs.all_combinations
run: cd xtask && cargo run -- check ${{ matrix.chip }}

- name: Generate and check project (all combinations)
if: github.event_name == 'schedule' || inputs.all_combinations
run: cd xtask && cargo run -- check ${{ matrix.chip }} --all-combinations


test:
runs-on: ubuntu-latest

Expand Down
44 changes: 38 additions & 6 deletions xtask/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ enum Commands {
/// Target chip to check
#[arg(value_enum)]
chip: Chip,
/// Verify all possible options combinations
#[arg(short, long)]
all_combinations: bool,
},
}

Expand All @@ -35,23 +38,27 @@ fn main() -> Result<()> {
let workspace = workspace.parent().unwrap().canonicalize()?;

match Cli::parse().command {
Commands::Check { chip } => check(&workspace, chip),
Commands::Check {
chip,
all_combinations,
} => check(&workspace, chip, all_combinations),
}
}

// ----------------------------------------------------------------------------
// CHECK

fn check(workspace: &Path, chip: Chip) -> Result<()> {
fn check(workspace: &Path, chip: Chip, all_combinations: bool) -> Result<()> {
log::info!("CHECK: {chip}");

const PROJECT_NAME: &str = "test";
for options in options_for_chip(chip) {
for options in options_for_chip(chip, all_combinations) {
log::info!("WITH OPTIONS: {options:?}");

// We will generate the project in a temporary directory, to avoid
// making a mess when this subcommand is executed locally:
let project_path = tempfile::tempdir()?.into_path();
let project_dir = tempfile::tempdir()?;
let project_path = project_dir.path();
log::info!("PROJECT PATH: {project_path:?}");

// Generate a project targeting the specified chip and using the
Expand Down Expand Up @@ -81,12 +88,14 @@ fn check(workspace: &Path, chip: Chip) -> Result<()> {
.stdout(Stdio::inherit())
.stderr(Stdio::inherit())
.output()?;

project_dir.close()?;
}

Ok(())
}

fn options_for_chip(chip: Chip) -> Vec<Vec<String>> {
fn options_for_chip(chip: Chip, all_combinations: bool) -> Vec<Vec<String>> {
let default_options: Vec<Vec<String>> = vec![
vec![], // No options
vec!["alloc".into()],
Expand All @@ -96,7 +105,7 @@ fn options_for_chip(chip: Chip) -> Vec<Vec<String>> {
vec!["probe-rs".into()],
];

match chip {
let available_options = match chip {
Chip::Esp32h2 => default_options
.iter()
.filter(|opts| !opts.contains(&"wifi".to_string()))
Expand All @@ -108,6 +117,29 @@ fn options_for_chip(chip: Chip) -> Vec<Vec<String>> {
.cloned()
.collect::<Vec<_>>(),
_ => default_options,
};
if !all_combinations {
return available_options;
} else {
// Return all the combination of availble options
let mut result = vec![];
for i in 0..(1 << available_options.len()) {
let mut options = vec![];
for j in 0..available_options.len() {
if i & (1 << j) != 0 {
options.extend(available_options[j].clone());
}
}
result.push(options);
}
// Filter all the items that contains wifi and ble
let result = result
.into_iter()
.filter(|opts| {
!opts.contains(&"wifi".to_string()) || !opts.contains(&"ble".to_string())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the purpose of this filter?

Copy link
Member Author

@SergioGasquez SergioGasquez Oct 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, when using ble and wifi, we throw an error: https://github.com/esp-rs/esp-generate/blob/main/src/main.rs#L408.

We should decide if we want to close the issue, and keep it like this. Or add support to allow using both options at the same time.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can fix this later, but we should definitely fix it as they aren't really mutually exclusive.

Probably worth fixing after lands: esp-rs/esp-hal#2301 as it should simplify initializing things

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can fix this later, but we should definitely fix it as they aren't really mutually exclusive.

FYI, the TUI does not allow selecting both, this was a temporary fix to resolve #19. See this comment.

})
.collect::<Vec<_>>();
return result;
}
}

Expand Down