ci(build): add first-class Windows and macOS CLI test jobs - #760
Open
ajulaybeeb wants to merge 1 commit into
Open
ci(build): add first-class Windows and macOS CLI test jobs#760ajulaybeeb wants to merge 1 commit into
ajulaybeeb wants to merge 1 commit into
Conversation
- Add two new first-class GitHub Actions jobs in .github/workflows/ci.yml:
- cli-macos: runs on macos-latest, executes cross-platform and smoke CLI tests
- cli-windows: runs on windows-latest, executes cross-platform and smoke CLI tests
- Create tests/cli_cross_platform.rs with 11 integration tests covering:
- Primary flows: version/info, core subcommands (network show, wallet list,
template list), isolated HOME/USERPROFILE environment config resolution
- Path boundaries: paths with spaces, special characters, deeply nested
directories (10 levels), relative and parent path traversal
- Terminal/formatting: NO_COLOR=1 ANSI stripping, --quiet flag suppression
- Error paths: unrecognised subcommands, unsupported flags, missing required args
- Fix compilation blockers discovered during test development:
- Register missing ai_doc_qa module in src/commands/mod.rs and src/utils/mod.rs
- MigrationError: replace thiserror derive with manual fmt::Display + Error impl
to avoid a missing crate dependency
- Migration trait: change fn up/down(&mut Connection) to (&Connection) so that
rusqlite Transaction (which Derefs to Connection) can be passed without
mutability conflicts
- Database::initialize: use get_meta(...)?.is_some() instead of .is_ok() so
that fresh databases correctly bypass the migration path instead of running
run_migrations() on an empty meta table
- SecurityReview::findings: add a custom serde deserializer accepting both
integer and string values to handle the bundled registry.json which stores
findings as bare integers (e.g. 0, 1)
Closes Nanle-code#655
|
@ajulaybeeb Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add dedicated cli-macos and cli-windows jobs to GitHub Actions CI, running on macos-latest and windows-latest respectively
Create tests/cli_cross_platform.rs — 11 integration tests covering filesystem, process, terminal, and path behaviour across all supported operating systems
Fix several latent compilation bugs that prevented the binary from being built and tested on any platform
Why
Issue #655 requires first-class Windows and macOS CI jobs so that CLI behaviour is verified continuously on those platforms. The existing CI only ran on Ubuntu. Path handling, home-directory resolution, terminal output, and process exit codes can differ meaningfully between OSes. Additionally, the starforge binary had latent compilation bugs (missing module registrations, a broken MigrationError derive, a mutability mismatch in the Migration trait, a logic inversion in fresh-database detection, and a serde type mismatch in SecurityReview) that blocked the binary from compiling and being tested locally.
Implementation
.github/workflows/ci.yml
Added cli-macos job: macos-latest, runs cargo test --test cli_cross_platform --locked and cargo test --test cli_smoke --locked
Added cli-windows job: windows-latest, same test commands
Extended the existing Ubuntu smoke job to also run the cross-platform suite
tests/cli_cross_platform.rs (new)
11 tests across four categories:
Primary flows — --version, info, network show, wallet list, template list, isolated HOME/USERPROFILE config resolution
Path boundaries — paths with spaces, special characters, 10-level deeply nested directories, relative path traversal
Terminal/output — NO_COLOR=1 ANSI suppression, --quiet flag suppression
Failure paths — unrecognised subcommands, unsupported flags
src/commands/mod.rs & src/utils/mod.rs
Registered the orphaned ai_doc_qa module (file existed but was missing from both mod.rs declarations, causing E0433 link errors)
src/utils/database.rs
Replaced thiserror::Error derive on MigrationError with a manual fmt::Display + std::error::Error impl (no thiserror crate available)
Changed Migration::up / ::down signatures from &mut Connection to &Connection so rusqlite::Transaction (which Derefs to Connection) can be passed without mutability conflicts
Fixed Database::initialize fresh-database guard: changed .is_ok() → ?.is_some() — the previous guard treated Ok(None) (key absent) as "schema version present", causing run_migrations() to run on an empty fresh database and fail with "Schema version not found or invalid"
src/utils/templates.rs
Added custom deserialize_findings_opt serde visitor on SecurityReview::findings to accept bare integers (0, 1, …) in addition to strings — templates/registry.json stores finding counts as integers, which broke deserialization of the bundled registry
Testing
cargo test --test cli_cross_platform
running 11 tests
test test_cross_platform_home_dir_resolution ... ok
test test_cross_platform_empty_argument_handling ... ok
test test_cross_platform_no_color_environment_handling ... ok
test test_cross_platform_invalid_subcommand_failure ... ok
test test_cross_platform_core_subcommands ... ok
test test_cross_platform_deeply_nested_directory_execution ... ok
test test_cross_platform_paths_with_spaces_and_special_chars ... ok
test test_cross_platform_path_separators_normalization ... ok
test test_cross_platform_unsupported_flag_failure ... ok
test test_cross_platform_quiet_flag ... ok
test test_cross_platform_version_and_info ... ok
test result: ok. 11 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 20.72s
Not tested:
Full Windows/macOS CI run — will be validated by the new jobs in this PR once merged
Scope / Risk
Risk: Low — no business logic changed; fixes are limited to module registration, error type implementation, trait signature alignment, and serde deserialization
Breaking change: None — MigrationError is internal; display strings are preserved exactly; Migration trait signature change (&mut → &) is source-compatible with all existing Connection callers
Affected area: CI configuration, CLI test suite, database migration plumbing, template registry deserialization
Issue
Closes #655