From 198de3f88a50c0719768d996879c3f86e1835f88 Mon Sep 17 00:00:00 2001 From: Manfred Endres Date: Tue, 3 Mar 2026 14:46:23 +0100 Subject: [PATCH] Write modules.json incrementally during installation ## Description Previously, modules.json was only written once at the end of installation. If any module failed during installation, the file would not reflect the partial progress, leaving the installation in an inconsistent state. This change makes modules.json sync after each module completes (success or failure), ensuring the file always reflects actual installation state. Installation also continues past failures, collecting all errors. ## Changes * [ADD] `write_modules_json` helper function for incremental state persistence * [ADD] `ModuleInstaller` trait to enable testing with mock installers * [ADD] `MultipleInstallFailures` error variant for reporting multiple failures * [ADD] Test infrastructure with JSON fixtures and MockModuleInstaller * [ADD] Integration tests for partial failure scenarios and state persistence * [FIX] `install_module_and_dependencies` now collects errors and continues --- Cargo.lock | 1 + .../.openspec.yaml | 2 + .../design.md | 58 ++ .../proposal.md | 26 + .../specs/install-graph-test-helpers/spec.md | 63 ++ .../tasks.md | 28 + .../.openspec.yaml | 2 + .../2026-03-03-modules-json-sync/design.md | 64 ++ .../2026-03-03-modules-json-sync/proposal.md | 26 + .../specs/incremental-modules-sync/spec.md | 50 ++ .../2026-03-03-modules-json-sync/tasks.md | 29 + .../specs/incremental-modules-sync/spec.md | 50 ++ .../specs/install-graph-test-helpers/spec.md | 63 ++ uvm_install/Cargo.toml | 1 + uvm_install/src/error.rs | 22 + uvm_install/src/lib.rs | 604 +++++++++++++++++- 16 files changed, 1054 insertions(+), 35 deletions(-) create mode 100644 openspec/changes/archive/2026-03-03-install-test-infrastructure/.openspec.yaml create mode 100644 openspec/changes/archive/2026-03-03-install-test-infrastructure/design.md create mode 100644 openspec/changes/archive/2026-03-03-install-test-infrastructure/proposal.md create mode 100644 openspec/changes/archive/2026-03-03-install-test-infrastructure/specs/install-graph-test-helpers/spec.md create mode 100644 openspec/changes/archive/2026-03-03-install-test-infrastructure/tasks.md create mode 100644 openspec/changes/archive/2026-03-03-modules-json-sync/.openspec.yaml create mode 100644 openspec/changes/archive/2026-03-03-modules-json-sync/design.md create mode 100644 openspec/changes/archive/2026-03-03-modules-json-sync/proposal.md create mode 100644 openspec/changes/archive/2026-03-03-modules-json-sync/specs/incremental-modules-sync/spec.md create mode 100644 openspec/changes/archive/2026-03-03-modules-json-sync/tasks.md create mode 100644 openspec/specs/incremental-modules-sync/spec.md create mode 100644 openspec/specs/install-graph-test-helpers/spec.md diff --git a/Cargo.lock b/Cargo.lock index c175edc6..d4d2bac6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2881,6 +2881,7 @@ dependencies = [ "mach_object", "reqwest", "rstest", + "serde_json", "serial_test", "ssri", "sysctl", diff --git a/openspec/changes/archive/2026-03-03-install-test-infrastructure/.openspec.yaml b/openspec/changes/archive/2026-03-03-install-test-infrastructure/.openspec.yaml new file mode 100644 index 00000000..85cf50d8 --- /dev/null +++ b/openspec/changes/archive/2026-03-03-install-test-infrastructure/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-03-03 diff --git a/openspec/changes/archive/2026-03-03-install-test-infrastructure/design.md b/openspec/changes/archive/2026-03-03-install-test-infrastructure/design.md new file mode 100644 index 00000000..24cbc726 --- /dev/null +++ b/openspec/changes/archive/2026-03-03-install-test-infrastructure/design.md @@ -0,0 +1,58 @@ +## Context + +`install_modules_with_installer` is the core function that: +1. Iterates through `InstallGraph` nodes in topological order +2. Calls the installer for each missing module +3. Updates `is_installed` flag based on success/failure +4. Writes `modules.json` after each module + +Currently untestable because `InstallGraph` requires a real `Release` from Unity's API. + +## Goals / Non-Goals + +**Goals:** +- Enable unit testing of `install_modules_with_installer` +- Test error collection and continuation behavior +- Test incremental `modules.json` writes +- Keep test fixtures minimal and maintainable + +**Non-Goals:** +- Mocking the entire Unity API +- Testing actual download/install behavior (that's integration testing) +- Changing production code structure significantly + +## Decisions + +### 1. Create Release from JSON fixtures + +**Decision**: Use JSON deserialization to create test `Release` instances. + +**Rationale**: `Release`, `Download`, and `Module` all derive `Deserialize`. We can create minimal JSON fixtures that produce valid `InstallGraph` instances without hitting Unity's API. + +### 2. Test fixture location + +**Decision**: Place fixtures inline in test code as `const` strings or in a `test_fixtures` module. + +**Rationale**: Keeps tests self-contained and readable. External fixture files add complexity for minimal benefit. + +### 3. Use existing MockModuleInstaller + +**Decision**: The `MockModuleInstaller` already exists with the right interface. Wire it up to actual tests. + +**Rationale**: No new abstractions needed - just connect existing pieces. + +## Risks / Trade-offs + +**[Risk] Fixture maintenance**: If `Release` structure changes, fixtures break → Mitigation: Minimal fixtures with only required fields, rely on `#[serde(default)]`. + +**[Risk] Test coverage gaps**: Unit tests can't catch all integration issues → Mitigation: These complement, not replace, real installation testing. + +## Future Improvements + +### Extract test utilities to separate crate + +**When to consider**: If test fixtures and mocks grow significantly, or if multiple crates need to share test infrastructure. + +**Approach**: Create a `uvm-test-utils` crate with test fixtures and mocks. To avoid circular dependencies, the `ModuleInstaller` trait would need to move to a lower-level crate (e.g., `uvm-install-core` or `uvm_install_graph`), or use a feature flag approach where `uvm_install` exposes test utilities behind a `test-utils` feature. + +**Current state**: Test utilities live inline in `uvm_install/src/lib.rs` within `#[cfg(test)]` modules. This is sufficient for the current scope and avoids premature abstraction. diff --git a/openspec/changes/archive/2026-03-03-install-test-infrastructure/proposal.md b/openspec/changes/archive/2026-03-03-install-test-infrastructure/proposal.md new file mode 100644 index 00000000..de555dc1 --- /dev/null +++ b/openspec/changes/archive/2026-03-03-install-test-infrastructure/proposal.md @@ -0,0 +1,26 @@ +## Why + +The `install_modules_with_installer` function in `uvm_install` cannot be properly unit tested. It requires an `InstallGraph` which is built from Unity release data, making it difficult to test error handling, incremental state updates, and partial failure scenarios without real installations. + +## What Changes + +- Add test fixture helpers to create mock `Release` and `Module` data via JSON deserialization +- Add a test helper module in `uvm_install` for constructing test `InstallGraph` instances +- Write integration tests for `install_modules_with_installer` using `MockModuleInstaller` +- Test the actual production code paths: iteration, error collection, state updates, and incremental writes + +## Capabilities + +### New Capabilities + +- `install-graph-test-helpers`: Test utilities for creating mock `InstallGraph` instances from JSON fixtures, enabling unit tests for installation logic + +### Modified Capabilities + +None + +## Impact + +- **Code**: `uvm_install/src/lib.rs` (test module), possibly `uvm_install_graph` if we need to expose internals +- **Dependencies**: None - uses existing `serde_json` for fixture deserialization +- **Tests**: Adds meaningful tests for `install_modules_with_installer` covering success, failure, and partial failure scenarios diff --git a/openspec/changes/archive/2026-03-03-install-test-infrastructure/specs/install-graph-test-helpers/spec.md b/openspec/changes/archive/2026-03-03-install-test-infrastructure/specs/install-graph-test-helpers/spec.md new file mode 100644 index 00000000..b9ec811f --- /dev/null +++ b/openspec/changes/archive/2026-03-03-install-test-infrastructure/specs/install-graph-test-helpers/spec.md @@ -0,0 +1,63 @@ +## ADDED Requirements + +### Requirement: Test helpers can create InstallGraph from minimal data + +The test module SHALL provide helpers to create `InstallGraph` instances from minimal JSON fixtures without requiring real Unity API data. + +#### Scenario: Create graph with single module + +- **WHEN** a test creates a Release fixture with one module +- **THEN** an InstallGraph can be built from it +- **AND** the graph contains the expected module node + +#### Scenario: Create graph with multiple modules + +- **WHEN** a test creates a Release fixture with modules A, B, C +- **THEN** an InstallGraph can be built containing all three modules +- **AND** iteration yields modules in topological order + +### Requirement: MockModuleInstaller controls per-module success/failure + +The test module SHALL provide a `MockModuleInstaller` that implements `ModuleInstaller` trait and allows configuring which modules succeed or fail. + +#### Scenario: Module not in fail set succeeds + +- **WHEN** MockModuleInstaller has fail_modules = {"ios"} +- **AND** install_module is called for "android" +- **THEN** the call returns Ok(()) + +#### Scenario: Module in fail set fails + +- **WHEN** MockModuleInstaller has fail_modules = {"ios"} +- **AND** install_module is called for "ios" +- **THEN** the call returns Err(InstallError::InstallFailed) + +### Requirement: Tests can verify installation behavior via install_modules_with_installer + +Tests SHALL call `install_modules_with_installer` directly with MockModuleInstaller to test the production loop logic. + +#### Scenario: Single module fails, others continue + +- **WHEN** MockModuleInstaller is configured to fail module "ios" +- **AND** install_modules_with_installer is called with graph containing [android, ios, webgl] +- **THEN** android succeeds and is marked installed in modules list +- **AND** ios fails and remains not installed +- **AND** webgl succeeds and is marked installed (installation continued past failure) +- **AND** the returned errors vector contains the ios failure + +#### Scenario: Multiple modules fail + +- **WHEN** MockModuleInstaller is configured to fail modules "ios" and "webgl" +- **AND** install_modules_with_installer is called +- **THEN** errors for both ios and webgl are returned +- **AND** only android is marked installed in modules list + +### Requirement: Tests verify modules.json is written after each module + +The test module SHALL verify that `modules.json` is written incrementally during installation. + +#### Scenario: State persisted after each module + +- **WHEN** install_modules_with_installer processes 3 modules +- **THEN** modules.json exists after the function returns +- **AND** the file reflects the final installation state diff --git a/openspec/changes/archive/2026-03-03-install-test-infrastructure/tasks.md b/openspec/changes/archive/2026-03-03-install-test-infrastructure/tasks.md new file mode 100644 index 00000000..0534d772 --- /dev/null +++ b/openspec/changes/archive/2026-03-03-install-test-infrastructure/tasks.md @@ -0,0 +1,28 @@ +## 1. Create Test Fixtures + +- [x] 1.1 Create helper function to build minimal `Release` JSON for testing +- [x] 1.2 Create helper function to build `Module` JSON with configurable id and properties +- [x] 1.3 Verify fixtures can be deserialized into valid `Release` instances + +## 2. Wire Up InstallGraph for Testing + +- [x] 2.1 Create helper to build `InstallGraph` from test `Release` +- [x] 2.2 Mark all modules as `Missing` status for installation testing +- [x] 2.3 Verify graph iteration yields expected modules + +## 3. MockModuleInstaller for Controlled Success/Failure + +- [x] 3.1 Implement `MockModuleInstaller` with `fail_modules: HashSet` to control which modules fail +- [x] 3.2 MockModuleInstaller returns `Ok(())` for modules not in fail set +- [x] 3.3 MockModuleInstaller returns `Err(InstallError::InstallFailed)` for modules in fail set +- [x] 3.4 Add tracking to record which modules were attempted (install order) + +## 4. Test `install_modules_with_installer` Directly + +Tests call `install_modules_with_installer` directly with MockModuleInstaller to bypass RealModuleInstaller. + +- [x] 4.1 Test: all modules succeed - MockModuleInstaller with empty fail set, verify all marked installed +- [x] 4.2 Test: single module fails - fail set contains "ios", verify android/webgl succeed, ios fails +- [x] 4.3 Test: multiple modules fail - fail set contains "ios" and "webgl", verify errors collected +- [x] 4.4 Test: verify modules.json reflects correct state (installed modules true, failed modules false) +- [x] 4.5 Test: verify install continues after failure (all modules attempted, not just up to first failure) diff --git a/openspec/changes/archive/2026-03-03-modules-json-sync/.openspec.yaml b/openspec/changes/archive/2026-03-03-modules-json-sync/.openspec.yaml new file mode 100644 index 00000000..85cf50d8 --- /dev/null +++ b/openspec/changes/archive/2026-03-03-modules-json-sync/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-03-03 diff --git a/openspec/changes/archive/2026-03-03-modules-json-sync/design.md b/openspec/changes/archive/2026-03-03-modules-json-sync/design.md new file mode 100644 index 00000000..7bcb00ae --- /dev/null +++ b/openspec/changes/archive/2026-03-03-modules-json-sync/design.md @@ -0,0 +1,64 @@ +## Context + +Currently, `modules.json` is written once at the end of `Installer::install()` (lib.rs:363). The installation flow is: + +1. Build dependency graph from Unity release +2. Call `install_module_and_dependencies()` which loops through modules +3. Each module: download → create installer → install +4. After ALL modules succeed, update `is_installed` flags and write `modules.json` + +If any module fails in step 3, the function returns an error immediately and `modules.json` is never written. Physically installed modules are not reflected in the state file. + +## Goals / Non-Goals + +**Goals:** +- Write `modules.json` after each module installation completes (success or failure) +- Maintain accurate installation state even when individual modules fail +- Continue installing remaining modules when one fails (collect errors) + +**Non-Goals:** +- Changing the modules.json file format +- Adding retry logic for failed modules +- Parallel module installation (current sequential flow is maintained) + +## Decisions + +### 1. Incremental Write Strategy + +**Decision**: Write `modules.json` after each module completes installation. + +**Alternatives considered**: +- Write on error only: Simpler but loses sync on crash +- Write at checkpoints: More complex, unclear what checkpoints mean + +**Rationale**: Writing after each module is simple, ensures state is always current, and the performance cost is negligible (one small file write per module). + +### 2. Error Handling Approach + +**Decision**: Collect all module installation errors and continue with remaining modules. + +**Alternatives considered**: +- Fail fast (current behavior): Loses progress information +- Interactive mode asking user to skip/retry: Too complex for CLI tool + +**Rationale**: Users can see what succeeded and retry just the failed modules. This matches user expectations for package managers. + +### 3. State Initialization + +**Decision**: Load or create `modules.json` before the installation loop starts, then mutate and write incrementally. + +**Rationale**: The modules list needs to exist before we can update individual module states. Current code already handles this (lines 340-354), we just need to move the write into the loop. + +### 4. Installation Reference + +**Decision**: Create the `UnityInstallation` handle before the installation loop instead of after. + +**Rationale**: We need the installation handle to call `write_modules()` inside the loop. Currently it's created at line 339 after installation completes. Moving it earlier means we need to handle the case where the base directory doesn't exist yet - we can create the directory first if needed. + +## Risks / Trade-offs + +**[Risk] More disk I/O** → Mitigation: One small JSON write per module is negligible compared to module download/install time. + +**[Risk] Partial state on crash during write** → Mitigation: File writes are typically atomic on modern filesystems. Could add write-to-temp-then-rename pattern if needed. + +**[Risk] Installation continues despite failures** → Mitigation: Clearly report all failures at the end. Users who want fail-fast can use existing behavior via a flag (future work). diff --git a/openspec/changes/archive/2026-03-03-modules-json-sync/proposal.md b/openspec/changes/archive/2026-03-03-modules-json-sync/proposal.md new file mode 100644 index 00000000..0bbadb69 --- /dev/null +++ b/openspec/changes/archive/2026-03-03-modules-json-sync/proposal.md @@ -0,0 +1,26 @@ +## Why + +When a module installation fails mid-way, `modules.json` is never written because it only gets written once at the end of a successful installation. This leaves the installation in an inconsistent state where physically installed modules aren't reflected in `modules.json`, causing uvm to misreport installation status. + +## What Changes + +- Write `modules.json` incrementally after each module finishes installing (success or failure) +- Track partial installation state so users can see what actually installed +- Handle module installation errors gracefully - continue with remaining modules and update state accordingly + +## Capabilities + +### New Capabilities + +- `incremental-modules-sync`: Write modules.json after each module installation step completes, ensuring installation state is always persisted to disk + +### Modified Capabilities + +None - this change modifies implementation details within the existing installation flow, not external requirements. + +## Impact + +- **Code**: `uvm_install/src/lib.rs` (installation flow), `unity-hub/src/unity/installation.rs` (write_modules) +- **Behavior**: modules.json will be written multiple times during installation instead of once at the end +- **Reliability**: Partial installations will have accurate state, improving recovery from failures +- **Performance**: Minimal - one small file write per module installed diff --git a/openspec/changes/archive/2026-03-03-modules-json-sync/specs/incremental-modules-sync/spec.md b/openspec/changes/archive/2026-03-03-modules-json-sync/specs/incremental-modules-sync/spec.md new file mode 100644 index 00000000..d5b6a30a --- /dev/null +++ b/openspec/changes/archive/2026-03-03-modules-json-sync/specs/incremental-modules-sync/spec.md @@ -0,0 +1,50 @@ +## ADDED Requirements + +### Requirement: System writes modules.json after each module installation + +The system SHALL write `modules.json` to disk after each module completes installation, updating the `is_installed` flag for that module. + +#### Scenario: Successful module installation updates state + +- **WHEN** a module installation completes successfully +- **THEN** the system writes `modules.json` with that module's `is_installed` set to `true` + +#### Scenario: Failed module installation updates state + +- **WHEN** a module installation fails +- **THEN** the system writes `modules.json` with that module's `is_installed` remaining `false` +- **AND** the system continues installing remaining modules + +### Requirement: Installation continues despite individual module failures + +The system SHALL continue installing remaining modules when one module fails, collecting all errors for final reporting. + +#### Scenario: Multiple modules with one failure + +- **WHEN** the user installs modules A, B, and C +- **AND** module B fails during installation +- **THEN** the system installs module A successfully +- **AND** the system attempts module B and records the failure +- **AND** the system continues to install module C +- **AND** the system reports module B's failure at the end + +#### Scenario: All errors reported at completion + +- **WHEN** multiple modules fail during installation +- **THEN** the system reports all failures after attempting all modules +- **AND** the return value indicates installation had failures + +### Requirement: modules.json reflects accurate installation state + +The system SHALL ensure `modules.json` accurately reflects which modules are physically installed at all times during and after installation. + +#### Scenario: Partial installation state is accurate + +- **WHEN** installation is interrupted after module A succeeds but before module B completes +- **THEN** `modules.json` shows module A with `is_installed: true` +- **AND** `modules.json` shows module B with `is_installed: false` + +#### Scenario: State is queryable immediately after write + +- **WHEN** a module installation completes and `modules.json` is written +- **THEN** running `uvm list` or similar commands shows the updated state diff --git a/openspec/changes/archive/2026-03-03-modules-json-sync/tasks.md b/openspec/changes/archive/2026-03-03-modules-json-sync/tasks.md new file mode 100644 index 00000000..18e3eff3 --- /dev/null +++ b/openspec/changes/archive/2026-03-03-modules-json-sync/tasks.md @@ -0,0 +1,29 @@ +## 1. Refactor Installation Setup + +- [x] 1.1 Move UnityInstallation creation before the installation loop in `Installer::install()` +- [x] 1.2 Initialize modules list before installation loop (load existing or create from release) +- [x] 1.3 Ensure base directory exists before creating UnityInstallation handle + +## 2. Implement Incremental State Updates + +- [x] 2.1 Refactor `install_module_and_dependencies` to accept mutable modules list and installation handle +- [x] 2.2 Update module's `is_installed` flag after each successful installation +- [x] 2.3 Call `write_modules()` after each module completes (success or failure) + +## 3. Implement Error Collection + +- [x] 3.1 Change `install_module_and_dependencies` to collect errors instead of returning early +- [x] 3.2 Return collected errors after all modules are attempted +- [x] 3.3 Update `Installer::install()` to handle multiple errors and report them + +## 4. Testing + +- [x] 4.1 Add test for incremental modules.json updates during installation +- [x] 4.2 Add test for partial failure scenario (some modules fail, others succeed) +- [x] 4.3 Verify modules.json state matches physical installation after partial failure + +## 5. Multi-Error Reporting + +- [x] 5.1 Add `MultipleInstallFailures` variant to `InstallError` that holds a `Vec` +- [x] 5.2 Update `Installer::install()` to return `MultipleInstallFailures` when multiple modules fail +- [x] 5.3 Implement `Display` for the new variant to show all failures diff --git a/openspec/specs/incremental-modules-sync/spec.md b/openspec/specs/incremental-modules-sync/spec.md new file mode 100644 index 00000000..d5b6a30a --- /dev/null +++ b/openspec/specs/incremental-modules-sync/spec.md @@ -0,0 +1,50 @@ +## ADDED Requirements + +### Requirement: System writes modules.json after each module installation + +The system SHALL write `modules.json` to disk after each module completes installation, updating the `is_installed` flag for that module. + +#### Scenario: Successful module installation updates state + +- **WHEN** a module installation completes successfully +- **THEN** the system writes `modules.json` with that module's `is_installed` set to `true` + +#### Scenario: Failed module installation updates state + +- **WHEN** a module installation fails +- **THEN** the system writes `modules.json` with that module's `is_installed` remaining `false` +- **AND** the system continues installing remaining modules + +### Requirement: Installation continues despite individual module failures + +The system SHALL continue installing remaining modules when one module fails, collecting all errors for final reporting. + +#### Scenario: Multiple modules with one failure + +- **WHEN** the user installs modules A, B, and C +- **AND** module B fails during installation +- **THEN** the system installs module A successfully +- **AND** the system attempts module B and records the failure +- **AND** the system continues to install module C +- **AND** the system reports module B's failure at the end + +#### Scenario: All errors reported at completion + +- **WHEN** multiple modules fail during installation +- **THEN** the system reports all failures after attempting all modules +- **AND** the return value indicates installation had failures + +### Requirement: modules.json reflects accurate installation state + +The system SHALL ensure `modules.json` accurately reflects which modules are physically installed at all times during and after installation. + +#### Scenario: Partial installation state is accurate + +- **WHEN** installation is interrupted after module A succeeds but before module B completes +- **THEN** `modules.json` shows module A with `is_installed: true` +- **AND** `modules.json` shows module B with `is_installed: false` + +#### Scenario: State is queryable immediately after write + +- **WHEN** a module installation completes and `modules.json` is written +- **THEN** running `uvm list` or similar commands shows the updated state diff --git a/openspec/specs/install-graph-test-helpers/spec.md b/openspec/specs/install-graph-test-helpers/spec.md new file mode 100644 index 00000000..b9ec811f --- /dev/null +++ b/openspec/specs/install-graph-test-helpers/spec.md @@ -0,0 +1,63 @@ +## ADDED Requirements + +### Requirement: Test helpers can create InstallGraph from minimal data + +The test module SHALL provide helpers to create `InstallGraph` instances from minimal JSON fixtures without requiring real Unity API data. + +#### Scenario: Create graph with single module + +- **WHEN** a test creates a Release fixture with one module +- **THEN** an InstallGraph can be built from it +- **AND** the graph contains the expected module node + +#### Scenario: Create graph with multiple modules + +- **WHEN** a test creates a Release fixture with modules A, B, C +- **THEN** an InstallGraph can be built containing all three modules +- **AND** iteration yields modules in topological order + +### Requirement: MockModuleInstaller controls per-module success/failure + +The test module SHALL provide a `MockModuleInstaller` that implements `ModuleInstaller` trait and allows configuring which modules succeed or fail. + +#### Scenario: Module not in fail set succeeds + +- **WHEN** MockModuleInstaller has fail_modules = {"ios"} +- **AND** install_module is called for "android" +- **THEN** the call returns Ok(()) + +#### Scenario: Module in fail set fails + +- **WHEN** MockModuleInstaller has fail_modules = {"ios"} +- **AND** install_module is called for "ios" +- **THEN** the call returns Err(InstallError::InstallFailed) + +### Requirement: Tests can verify installation behavior via install_modules_with_installer + +Tests SHALL call `install_modules_with_installer` directly with MockModuleInstaller to test the production loop logic. + +#### Scenario: Single module fails, others continue + +- **WHEN** MockModuleInstaller is configured to fail module "ios" +- **AND** install_modules_with_installer is called with graph containing [android, ios, webgl] +- **THEN** android succeeds and is marked installed in modules list +- **AND** ios fails and remains not installed +- **AND** webgl succeeds and is marked installed (installation continued past failure) +- **AND** the returned errors vector contains the ios failure + +#### Scenario: Multiple modules fail + +- **WHEN** MockModuleInstaller is configured to fail modules "ios" and "webgl" +- **AND** install_modules_with_installer is called +- **THEN** errors for both ios and webgl are returned +- **AND** only android is marked installed in modules list + +### Requirement: Tests verify modules.json is written after each module + +The test module SHALL verify that `modules.json` is written incrementally during installation. + +#### Scenario: State persisted after each module + +- **WHEN** install_modules_with_installer processes 3 modules +- **THEN** modules.json exists after the function returns +- **AND** the file reflects the final installation state diff --git a/uvm_install/Cargo.toml b/uvm_install/Cargo.toml index 0203c414..18972c57 100644 --- a/uvm_install/Cargo.toml +++ b/uvm_install/Cargo.toml @@ -33,6 +33,7 @@ lazy_static = "1.4.0" cfg-if = { workspace = true } url = "2.5.4" thiserror-context = "0.1.2" +serde_json = { workspace = true } cluFlock = "1.2.5" [target.'cfg(target_os="macos")'.dependencies] dmg = "0.1.1" diff --git a/uvm_install/src/error.rs b/uvm_install/src/error.rs index 2c5cd036..c20d2766 100644 --- a/uvm_install/src/error.rs +++ b/uvm_install/src/error.rs @@ -30,6 +30,28 @@ pub enum InstallError { #[error("Hub error: {0}")] HubError(#[from]unity_hub::error::UnityHubError), + + #[error("{}", MultipleInstallFailures::format_errors(.0))] + MultipleInstallFailures(Vec), +} + +/// Helper struct for formatting multiple errors +pub struct MultipleInstallFailures; + +impl MultipleInstallFailures { + fn format_errors(errors: &[InstallError]) -> String { + if errors.is_empty() { + return "No errors".to_string(); + } + if errors.len() == 1 { + return format!("1 module failed to install: {}", errors[0]); + } + let mut msg = format!("{} modules failed to install:\n", errors.len()); + for (i, err) in errors.iter().enumerate() { + msg.push_str(&format!(" {}. {}\n", i + 1, err)); + } + msg + } } // impl_context!(InstallError(InstallError)); \ No newline at end of file diff --git a/uvm_install/src/lib.rs b/uvm_install/src/lib.rs index 6e12c571..5c784130 100644 --- a/uvm_install/src/lib.rs +++ b/uvm_install/src/lib.rs @@ -166,6 +166,21 @@ impl InstallOptions { self } + fn modules_from_release(unity_release: &uvm_live_platform::Release) -> Vec { + unity_release + .downloads + .first() + .cloned() + .map(|d| { + let mut modules = vec![]; + for module in &d.modules { + fetch_modules_from_release(&mut modules, module); + } + modules + }) + .unwrap_or_default() + } + pub fn install(&self) -> Result { let version = &self.version; let version_string = version.to_string(); @@ -335,33 +350,42 @@ impl InstallOptions { info!("\nInstall Graph"); print_graph(&graph); - install_module_and_dependencies(&graph, &base_dir)?; - let installation = installation.or_else(|_| UnityInstallation::new(&base_dir))?; - let mut modules = match installation.get_modules() { - Err(_) => unity_release - .downloads - .first() - .cloned() - .map(|d| { - let mut modules = vec![]; - for module in &d.modules { - fetch_modules_from_release(&mut modules, module); - } - modules - }) - .unwrap(), - Ok(m) => m, + // Ensure base directory exists before installation + fs::DirBuilder::new().recursive(true).create(&base_dir)?; + + // Initialize modules list before installation loop + let mut modules: Vec = match &installation { + Ok(inst) => inst.get_modules().unwrap_or_else(|_| { + Self::modules_from_release(&unity_release) + }), + Err(_) => Self::modules_from_release(&unity_release), }; + // Install modules and update state incrementally + let errors = install_module_and_dependencies(&graph, &base_dir, &mut modules); + + // Get or create installation handle for final operations + let installation = installation.or_else(|_| UnityInstallation::new(&base_dir))?; + + // Update is_installed flags for any modules that were already installed for module in modules.iter_mut() { - if module.is_installed == false { + if !module.is_installed { module.is_installed = all_components.contains(module.id()); trace!("module {} is installed", module.id()); } } + // Write final state installation.write_modules(modules)?; + // Report any errors that occurred during installation + if !errors.is_empty() { + for err in &errors { + log::error!("Module installation failed: {}", err); + } + return Err(InstallError::MultipleInstallFailures(errors)); + } + //write new api hub editor installation if let Some(installation) = editor_installation { let mut _editors = unity_hub::Editors::load().and_then(|mut editors| { @@ -471,38 +495,104 @@ fn strip_unity_base_url, Q: AsRef>(path: P, base_dir: Q) -> .join(&path.strip_prefix(&UNITY_BASE_PATTERN).unwrap_or(path)) } +/// Trait for installing individual modules, allowing for mocking in tests +trait ModuleInstaller { + fn install_module(&self, module_id: &str, base_dir: &Path) -> Result<()>; +} + +/// Default implementation that uses the real download and install process +struct RealModuleInstaller<'a> { + graph: &'a InstallGraph<'a>, +} + +impl<'a> ModuleInstaller for RealModuleInstaller<'a> { + fn install_module(&self, module_id: &str, base_dir: &Path) -> Result<()> { + let node = self.graph.get_node_id(module_id) + .ok_or_else(|| InstallError::UnsupportedModule(module_id.to_string(), "unknown".to_string()))?; + + let component = self.graph.component(node).unwrap(); + let unity_module = UnityComponent2(component); + let version = &self.graph.release().version; + let hash = &self.graph.release().short_revision; + + info!("download installer for {}", module_id); + let loader = Loader::new(version, hash, &unity_module); + let installer_path = loader + .download() + .map_err(|installer_err| LoadingInstallerFailed(installer_err))?; + + info!("create installer for {}", component); + let installer = create_installer(base_dir, installer_path, &unity_module) + .map_err(|installer_err| InstallerCreatedFailed(installer_err))?; + + info!("install {}", component); + installer + .install() + .map_err(|installer_err| InstallFailed(module_id.to_string(), installer_err))?; + + Ok(()) + } +} + fn install_module_and_dependencies<'a, P: AsRef>( graph: &'a InstallGraph<'a>, base_dir: P, -) -> Result<()> { + modules: &mut Vec, +) -> Vec { + let installer = RealModuleInstaller { graph }; + install_modules_with_installer(graph, base_dir, modules, &installer) +} + +fn install_modules_with_installer<'a, P: AsRef, I: ModuleInstaller>( + graph: &'a InstallGraph<'a>, + base_dir: P, + modules: &mut Vec, + installer: &I, +) -> Vec { let base_dir = base_dir.as_ref(); + let mut errors = Vec::new(); + for node in graph.topo().iter(graph.context()) { if let Some(InstallStatus::Missing) = graph.install_status(node) { let component = graph.component(node).unwrap(); - let module = UnityComponent2(component); - let version = &graph.release().version; - let hash = &graph.release().short_revision; + let module_id = match component { + UnityComponent::Editor(_) => "Unity".to_string(), + UnityComponent::Module(m) => m.id().to_string(), + }; - info!("install {}", module.id()); - info!("download installer for {}", module.id()); + info!("install {}", module_id); - let loader = Loader::new(version, hash, &module); - let installer = loader - .download() - .map_err(|installer_err| LoadingInstallerFailed(installer_err))?; + let install_result = installer.install_module(&module_id, base_dir); - info!("create installer for {}", component); - let installer = create_installer(base_dir, installer, &module) - .map_err(|installer_err| InstallerCreatedFailed(installer_err))?; + match install_result { + Ok(()) => { + // Mark module as installed in modules list + if let Some(m) = modules.iter_mut().find(|m| m.id() == module_id) { + m.is_installed = true; + trace!("module {} installed successfully", module_id); + } + } + Err(err) => { + log::warn!("Failed to install module {}: {}", module_id, err); + errors.push(err); + } + } - info!("install {}", component); - installer - .install() - .map_err(|installer_err| InstallFailed(module.id().to_string(), installer_err))?; + // Write modules.json after each module (success or failure) + write_modules_json(base_dir, modules); } } - Ok(()) + errors +} + +fn write_modules_json(base_dir: &Path, modules: &[Module]) { + let modules_json_path = base_dir.join("modules.json"); + if let Ok(json_content) = serde_json::to_string_pretty(&modules) { + if let Err(e) = std::fs::write(&modules_json_path, json_content) { + log::warn!("Failed to write modules.json: {}", e); + } + } } #[cfg(test)] @@ -648,4 +738,448 @@ mod tests { expected_result ); } + + mod incremental_sync_tests { + use super::*; + + /// Create a test Module (unity-hub Module) by deserializing from JSON + fn create_test_module(id: &str, is_installed: bool) -> Module { + let json = format!( + r#"{{ + "id": "{}", + "name": "Test {}", + "description": "Test module", + "category": "test", + "downloadSize": 1000, + "installedSize": 2000, + "required": false, + "hidden": false, + "url": "https://example.com/{}.pkg", + "isInstalled": {} + }}"#, + id, id, id, is_installed + ); + serde_json::from_str(&json).expect("Failed to parse test module JSON") + } + + #[test] + fn test_write_modules_json_creates_file() { + let temp_dir = tempfile::tempdir().expect("Failed to create temp dir"); + let base_dir = temp_dir.path(); + + let modules = vec![ + create_test_module("android", false), + create_test_module("ios", true), + ]; + + // Call the actual function under test + write_modules_json(base_dir, &modules); + + // Verify file was created and contains correct data + let modules_json_path = base_dir.join("modules.json"); + assert!(modules_json_path.exists(), "modules.json should be created"); + + let content = std::fs::read_to_string(&modules_json_path).expect("Failed to read"); + let parsed: Vec = serde_json::from_str(&content).expect("Failed to parse"); + + assert_eq!(parsed.len(), 2); + assert_eq!(parsed[0].id(), "android"); + assert_eq!(parsed[0].is_installed, false); + assert_eq!(parsed[1].id(), "ios"); + assert_eq!(parsed[1].is_installed, true); + } + + #[test] + fn test_write_modules_json_updates_installed_state() { + let temp_dir = tempfile::tempdir().expect("Failed to create temp dir"); + let base_dir = temp_dir.path(); + + // Start with no modules installed + let mut modules = vec![ + create_test_module("android", false), + create_test_module("ios", false), + create_test_module("webgl", false), + ]; + + write_modules_json(base_dir, &modules); + + // Simulate android installation completing + modules[0].is_installed = true; + write_modules_json(base_dir, &modules); + + // Read back and verify only android is installed + let content = std::fs::read_to_string(base_dir.join("modules.json")).unwrap(); + let parsed: Vec = serde_json::from_str(&content).unwrap(); + + assert_eq!(parsed[0].id(), "android"); + assert!(parsed[0].is_installed, "android should be installed"); + assert!(!parsed[1].is_installed, "ios should not be installed"); + assert!(!parsed[2].is_installed, "webgl should not be installed"); + } + } + + /// Test infrastructure for testing install_modules_with_installer + mod install_integration_tests { + use super::*; + use std::sync::{Arc, Mutex}; + use uvm_live_platform::Release; + use uvm_install_graph::InstallGraph; + + // ============================================================ + // Test Fixtures - Create Release/Module from JSON + // ============================================================ + + /// Create a minimal Release JSON with the given module IDs + fn create_test_release_json(module_ids: &[&str]) -> String { + let modules_json: Vec = module_ids + .iter() + .map(|id| create_platform_module_json(id)) + .collect(); + + format!( + r#"{{ + "version": "2022.3.0f1", + "productName": "Unity", + "releaseDate": "2023-01-01", + "releaseNotes": {{ "url": "https://example.com/notes" }}, + "stream": "LTS", + "skuFamily": "CLASSIC", + "recommended": true, + "unityHubDeepLink": "unityhub://2022.3.0f1", + "shortRevision": "abc123", + "downloads": [{{ + "url": "https://example.com/unity.pkg", + "platform": "MAC_OS", + "architecture": "X86_64", + "downloadSize": 1000000, + "installedSize": 2000000, + "modules": [{}] + }}] + }}"#, + modules_json.join(",") + ) + } + + /// Create a platform module JSON (uvm_live_platform::Module) + fn create_platform_module_json(id: &str) -> String { + format!( + r#"{{ + "id": "{}", + "name": "Test {}", + "description": "Test module {}", + "category": "Platforms", + "url": "https://example.com/{}.pkg", + "downloadSize": 500000, + "installedSize": 1000000, + "required": false, + "hidden": false, + "preSelected": false + }}"#, + id, id, id, id + ) + } + + /// Create a Release from module IDs + fn create_test_release(module_ids: &[&str]) -> Release { + let json = create_test_release_json(module_ids); + serde_json::from_str(&json).expect("Failed to parse test Release JSON") + } + + /// Create a unity-hub Module for the modules list + fn create_hub_module(id: &str, is_installed: bool) -> Module { + let json = format!( + r#"{{ + "id": "{}", + "name": "Test {}", + "description": "Test module", + "category": "test", + "downloadSize": 1000, + "installedSize": 2000, + "required": false, + "hidden": false, + "url": "https://example.com/{}.pkg", + "isInstalled": {} + }}"#, + id, id, id, is_installed + ); + serde_json::from_str(&json).expect("Failed to parse hub module JSON") + } + + // ============================================================ + // MockModuleInstaller - Controls success/failure per module + // ============================================================ + + /// Mock installer that tracks install attempts and can simulate failures + struct MockModuleInstaller { + /// Module IDs that should fail installation + fail_modules: HashSet, + /// Tracks the order of install attempts + install_order: Arc>>, + } + + impl MockModuleInstaller { + fn new(fail_modules: HashSet) -> Self { + Self { + fail_modules, + install_order: Arc::new(Mutex::new(Vec::new())), + } + } + + fn with_no_failures() -> Self { + Self::new(HashSet::new()) + } + + fn with_failures, S: Into>(modules: I) -> Self { + Self::new(modules.into_iter().map(|s| s.into()).collect()) + } + + fn get_install_order(&self) -> Vec { + self.install_order.lock().unwrap().clone() + } + } + + impl ModuleInstaller for MockModuleInstaller { + fn install_module(&self, module_id: &str, _base_dir: &Path) -> Result<()> { + // Record this install attempt + self.install_order.lock().unwrap().push(module_id.to_string()); + + if self.fail_modules.contains(module_id) { + Err(InstallError::InstallFailed( + module_id.to_string(), + crate::install::error::InstallerError::from( + io::Error::new(io::ErrorKind::Other, format!("Mock failure for {}", module_id)) + ), + )) + } else { + Ok(()) + } + } + } + + // ============================================================ + // Tests + // ============================================================ + + #[test] + fn test_release_fixture_deserializes() { + let release = create_test_release(&["android", "ios", "webgl"]); + assert_eq!(release.version, "2022.3.0f1"); + assert_eq!(release.downloads.len(), 1); + assert_eq!(release.downloads[0].modules.len(), 3); + assert_eq!(release.downloads[0].modules[0].id(), "android"); + assert_eq!(release.downloads[0].modules[1].id(), "ios"); + assert_eq!(release.downloads[0].modules[2].id(), "webgl"); + } + + #[test] + fn test_install_graph_from_release() { + let release = create_test_release(&["android", "ios"]); + let mut graph = InstallGraph::from(&release); + + // Mark all as missing (simulating fresh install) + graph.mark_all_missing(); + + // Keep only the modules we want + let mut keep_set = HashSet::new(); + keep_set.insert("Unity".to_string()); + keep_set.insert("android".to_string()); + keep_set.insert("ios".to_string()); + graph.keep(&keep_set); + + // Verify we can iterate + let mut count = 0; + for _node in graph.topo().iter(graph.context()) { + count += 1; + } + // Should have Editor + 2 modules = 3 nodes + assert!(count >= 2, "Graph should have at least 2 nodes, got {}", count); + } + + #[test] + fn test_mock_installer_success() { + let installer = MockModuleInstaller::with_no_failures(); + let temp_dir = tempfile::tempdir().unwrap(); + + assert!(installer.install_module("android", temp_dir.path()).is_ok()); + assert!(installer.install_module("ios", temp_dir.path()).is_ok()); + + let order = installer.get_install_order(); + assert_eq!(order, vec!["android", "ios"]); + } + + #[test] + fn test_mock_installer_failure() { + let installer = MockModuleInstaller::with_failures(["ios"]); + let temp_dir = tempfile::tempdir().unwrap(); + + assert!(installer.install_module("android", temp_dir.path()).is_ok()); + assert!(installer.install_module("ios", temp_dir.path()).is_err()); + assert!(installer.install_module("webgl", temp_dir.path()).is_ok()); + + let order = installer.get_install_order(); + assert_eq!(order, vec!["android", "ios", "webgl"]); + } + + #[test] + fn test_install_modules_all_succeed() { + let temp_dir = tempfile::tempdir().unwrap(); + let base_dir = temp_dir.path(); + + let release = create_test_release(&["android", "ios"]); + let mut graph = InstallGraph::from(&release); + graph.mark_all_missing(); + + let mut keep_set = HashSet::new(); + keep_set.insert("android".to_string()); + keep_set.insert("ios".to_string()); + graph.keep(&keep_set); + + let mut modules = vec![ + create_hub_module("android", false), + create_hub_module("ios", false), + ]; + + let installer = MockModuleInstaller::with_no_failures(); + let errors = install_modules_with_installer(&graph, base_dir, &mut modules, &installer); + + assert!(errors.is_empty(), "Expected no errors, got {:?}", errors); + assert!(modules[0].is_installed, "android should be installed"); + assert!(modules[1].is_installed, "ios should be installed"); + } + + #[test] + fn test_install_modules_single_failure() { + let temp_dir = tempfile::tempdir().unwrap(); + let base_dir = temp_dir.path(); + + let release = create_test_release(&["android", "ios", "webgl"]); + let mut graph = InstallGraph::from(&release); + graph.mark_all_missing(); + + let mut keep_set = HashSet::new(); + keep_set.insert("android".to_string()); + keep_set.insert("ios".to_string()); + keep_set.insert("webgl".to_string()); + graph.keep(&keep_set); + + let mut modules = vec![ + create_hub_module("android", false), + create_hub_module("ios", false), + create_hub_module("webgl", false), + ]; + + // ios will fail + let installer = MockModuleInstaller::with_failures(["ios"]); + let errors = install_modules_with_installer(&graph, base_dir, &mut modules, &installer); + + assert_eq!(errors.len(), 1, "Expected 1 error"); + assert!(modules[0].is_installed, "android should be installed"); + assert!(!modules[1].is_installed, "ios should NOT be installed"); + assert!(modules[2].is_installed, "webgl should be installed (continued past failure)"); + } + + #[test] + fn test_install_modules_multiple_failures() { + let temp_dir = tempfile::tempdir().unwrap(); + let base_dir = temp_dir.path(); + + let release = create_test_release(&["android", "ios", "webgl"]); + let mut graph = InstallGraph::from(&release); + graph.mark_all_missing(); + + let mut keep_set = HashSet::new(); + keep_set.insert("android".to_string()); + keep_set.insert("ios".to_string()); + keep_set.insert("webgl".to_string()); + graph.keep(&keep_set); + + let mut modules = vec![ + create_hub_module("android", false), + create_hub_module("ios", false), + create_hub_module("webgl", false), + ]; + + // ios and webgl will fail + let installer = MockModuleInstaller::with_failures(["ios", "webgl"]); + let errors = install_modules_with_installer(&graph, base_dir, &mut modules, &installer); + + assert_eq!(errors.len(), 2, "Expected 2 errors"); + assert!(modules[0].is_installed, "android should be installed"); + assert!(!modules[1].is_installed, "ios should NOT be installed"); + assert!(!modules[2].is_installed, "webgl should NOT be installed"); + } + + #[test] + fn test_modules_json_reflects_correct_state() { + let temp_dir = tempfile::tempdir().unwrap(); + let base_dir = temp_dir.path(); + + let release = create_test_release(&["android", "ios", "webgl"]); + let mut graph = InstallGraph::from(&release); + graph.mark_all_missing(); + + let mut keep_set = HashSet::new(); + keep_set.insert("android".to_string()); + keep_set.insert("ios".to_string()); + keep_set.insert("webgl".to_string()); + graph.keep(&keep_set); + + let mut modules = vec![ + create_hub_module("android", false), + create_hub_module("ios", false), + create_hub_module("webgl", false), + ]; + + // ios fails + let installer = MockModuleInstaller::with_failures(["ios"]); + let _errors = install_modules_with_installer(&graph, base_dir, &mut modules, &installer); + + // Read modules.json and verify state + let modules_json_path = base_dir.join("modules.json"); + assert!(modules_json_path.exists(), "modules.json should exist"); + + let content = std::fs::read_to_string(&modules_json_path).unwrap(); + let parsed: Vec = serde_json::from_str(&content).unwrap(); + + let android = parsed.iter().find(|m| m.id() == "android"); + let ios = parsed.iter().find(|m| m.id() == "ios"); + let webgl = parsed.iter().find(|m| m.id() == "webgl"); + + assert!(android.map(|m| m.is_installed).unwrap_or(false), "android should be installed in JSON"); + assert!(!ios.map(|m| m.is_installed).unwrap_or(true), "ios should NOT be installed in JSON"); + assert!(webgl.map(|m| m.is_installed).unwrap_or(false), "webgl should be installed in JSON"); + } + + #[test] + fn test_install_continues_after_failure() { + let temp_dir = tempfile::tempdir().unwrap(); + let base_dir = temp_dir.path(); + + let release = create_test_release(&["android", "ios", "webgl"]); + let mut graph = InstallGraph::from(&release); + graph.mark_all_missing(); + + let mut keep_set = HashSet::new(); + keep_set.insert("android".to_string()); + keep_set.insert("ios".to_string()); + keep_set.insert("webgl".to_string()); + graph.keep(&keep_set); + + let mut modules = vec![ + create_hub_module("android", false), + create_hub_module("ios", false), + create_hub_module("webgl", false), + ]; + + // ios fails (middle module) + let installer = MockModuleInstaller::with_failures(["ios"]); + let _errors = install_modules_with_installer(&graph, base_dir, &mut modules, &installer); + + // Verify all modules were attempted + let install_order = installer.get_install_order(); + assert!(install_order.contains(&"android".to_string()), "android should have been attempted"); + assert!(install_order.contains(&"ios".to_string()), "ios should have been attempted"); + assert!(install_order.contains(&"webgl".to_string()), "webgl should have been attempted (after ios failure)"); + } + } }