chore: enable cargo test in CI#87
Conversation
There was a problem hiding this comment.
Pull request overview
This PR enables running cargo test in CI and adjusts the codebase (primarily documentation and a few tests/constants) to reduce failures from doctests and test concurrency.
Changes:
- Added a dedicated GitHub Actions job to run
cargo test --all-targets --all-features. - Removed multiple docstring example blocks (likely to reduce/avoid doctest compilation/execution issues).
- Hardened
src/paths.rstests against cross-test env-var races by serializing env mutation with a mutex.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/ci.yml |
Adds a new cargo-test job running cargo test. |
src/version_info.rs |
Removes docstring example for write_version_file. |
src/run_id/persistence.rs |
Removes docstring examples for run-id save/load helpers. |
src/run_id/mod.rs |
Removes docstring example for generate_run_id. |
src/port_allocator.rs |
Removes docstring example for PortAllocator. |
src/paths.rs |
Serializes tests that mutate FOC_DEVNET_BASEDIR using a global mutex. |
src/embedded_assets.rs |
Removes docstring example for extract_mockusdfc_project. |
src/docker/command_logger.rs |
Removes docstring example for format_command. |
src/constants.rs |
Updates container name constants from foc-c-* to foc-*. |
src/commands/status/uptime.rs |
Removes docstring examples for status helpers. |
src/commands/status/running.rs |
Removes docstring examples; updates a parsing test’s sample container names. |
src/commands/status/git/repo_paths.rs |
Fixes doc example to match current Location::GitBranch { .. } shape. |
src/commands/status/git/git_info.rs |
Removes docstring example for get_git_info. |
src/commands/status/git/formatters.rs |
Removes docstring example for format_location_info. |
src/commands/start/step/mod.rs |
Removes large docstring example for SetupContext/step usage and save_command. |
src/commands/init/keys.rs |
Updates test expectation for number of generated keys. |
| /// Docker container names (base - will be prefixed with foc-c-<RUN_ID>- in practice) | ||
| pub const LOTUS_CONTAINER: &str = "foc-c-lotus"; | ||
| pub const LOTUS_MINER_CONTAINER: &str = "foc-c-lotus-miner"; | ||
| pub const BUILDER_CONTAINER: &str = "foc-c-builder"; | ||
| pub const YUGABYTE_CONTAINER: &str = "foc-c-yugabyte"; | ||
| pub const CURIO_CONTAINER: &str = "foc-c-curio"; | ||
| pub const PORTAINER_CONTAINER: &str = "foc-c-portainer"; | ||
| pub const LOTUS_CONTAINER: &str = "foc-lotus"; | ||
| pub const LOTUS_MINER_CONTAINER: &str = "foc-lotus-miner"; | ||
| pub const BUILDER_CONTAINER: &str = "foc-builder"; | ||
| pub const YUGABYTE_CONTAINER: &str = "foc-yugabyte"; | ||
| pub const CURIO_CONTAINER: &str = "foc-curio"; | ||
| pub const PORTAINER_CONTAINER: &str = "foc-portainer"; |
There was a problem hiding this comment.
The comment above these container name constants still says they’ll be prefixed with foc-c-<RUN_ID>-, but the codebase now uses foc-<RUN_ID>-<service> (and these constants are also used directly when no run ID is present). Please update this doc comment to reflect the actual naming scheme to avoid misleading future changes.
| fn test_extract_base_image_name() { | ||
| assert_eq!( | ||
| extract_base_image_name("foc-26jan02-1058_TizzyTike-lotus"), | ||
| extract_base_image_name("foc-20251215T2206_ZanyPip-lotus"), | ||
| crate::constants::LOTUS_CONTAINER | ||
| ); | ||
| assert_eq!( | ||
| extract_base_image_name("foc-26jan02-1058_TizzyTike-lotus-miner"), | ||
| extract_base_image_name("foc-20251215T2206_ZanyPip-lotus-miner"), | ||
| crate::constants::LOTUS_MINER_CONTAINER | ||
| ); | ||
| assert_eq!( | ||
| extract_base_image_name("foc-26jan02-1058_TizzyTike-curio-1"), | ||
| extract_base_image_name("foc-20251215T2206_ZanyPip-curio-1"), | ||
| crate::constants::CURIO_CONTAINER | ||
| ); |
There was a problem hiding this comment.
extract_base_image_name currently splits on - and assumes the run ID contains no -. However save_current_run_id/tests in run_id::persistence accept hyphenated run IDs (e.g. "251203-1246-test-wolf"), which would make container names like foc-<run_id>-lotus parse incorrectly and return the wrong base image name. Consider making extract_base_image_name robust to - in the run ID (e.g., parse from the end or match known service suffixes), and add a test case that covers a hyphenated run ID to prevent regressions.
| - name: Run tests | ||
| run: cargo test --all-targets --all-features |
There was a problem hiding this comment.
This cargo test invocation will also run doctests; the repo currently has at least one runnable doctest in src/commands/status/mod.rs (module-level "Usage" example) that calls status::status() and will fail on a clean CI environment (requires an initialized config). Either mark that doctest as no_run/ignore (preferred) or adjust CI to avoid running doctests until the docs are fixed, otherwise this job is likely to be flaky/fail consistently.
Enables Cargo test in CI, removes a bunch of examples in docstrings.