Skip to content

Conversation

@Johan-Liebert1
Copy link
Collaborator

Figuring out how testing framework works

@github-actions github-actions bot added the area/documentation Updates to the documentation label Dec 8, 2025
@bootc-bot bootc-bot bot requested a review from ckyrouac December 8, 2025 09:30
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request appears to be for exploring the testing framework, as indicated by the title and description. It includes several temporary changes, such as hardcoded test filters, debug print statements, commented-out functionality, and a new example test file. These changes should be reverted before merging. I've left specific comments on each of these points. The typo fix in docs/src/host-v1.schema.json is a good correction.

Comment on lines +782 to +785
if stem != "testing-ex" {
println!("SKIPPING: {stem}");
continue
}
Copy link
Contributor

Choose a reason for hiding this comment

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

high

This block of code hardcodes the test execution to only run for testing-ex. This appears to be for debugging and should be removed before merging to allow all tests to be processed.

fn update_generated(sh: &Shell) -> Result<()> {
// Update man pages (create new templates + sync options)
man::update_manpages(sh)?;
// man::update_manpages(sh)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

high

This function call appears to be commented out for testing. Please re-enable it before merging to ensure man pages are updated.

Suggested change
// man::update_manpages(sh)?;
man::update_manpages(sh)?;


// Update JSON schemas
update_json_schemas(sh)?;
// update_json_schemas(sh)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

high

This function call appears to be commented out for testing. Please re-enable it before merging to ensure JSON schemas are updated.

Suggested change
// update_json_schemas(sh)?;
update_json_schemas(sh)?;


let test_command = format!("{} {}", extension, relative_path.display());

println!("test_command: {test_command}");
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

This println! statement appears to be for debugging. Please remove it before merging.

//! by the user - add commands here which otherwise might
//! end up as a lot of nontrivial bash code.
#![allow(dead_code)]
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The crate-level #![allow(dead_code)] should be removed. While useful during development, it can hide real issues with unused code. If specific warnings need to be suppressed, it's better to apply the attribute to a more granular scope.

Comment on lines +25 to +28
**--all**

Print all configuration

Copy link
Contributor

Choose a reason for hiding this comment

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

medium

This content has been added inside a block that appears to be auto-generated (<!-- BEGIN GENERATED OPTIONS --> ... <!-- END GENERATED OPTIONS -->). Manual edits here will likely be overwritten. If this option needs to be documented, the change should be made in the generation logic instead.

Comment on lines +1 to +10
# number: 450
# tmt:
# summary: Some random test
# duration: 5m
#
# Verify that something works

def main[] {
echo "The test has passed"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

This appears to be a temporary test file created for experimentation. It should be removed before this pull request is merged to avoid adding unnecessary test code to the repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant