fix: error if explicit canister path is bad (#276)

Author: viviveevee

CHANGELOG.md

@@ -1,5 +1,7 @@
 # Unreleased
 
+* fix: Validate explicit canister paths and throw an error if `canister.yaml` is not found
+
 # v0.1.0-beta.3
 
 * feat: Remove requirement that the user install `icp-cli-network-launcher`, auto-install it on first use

crates/icp-cli/src/operations/settings.rs

@@ -12,6 +12,7 @@ use snafu::{ResultExt, Snafu};
 use crate::progress::{ProgressManager, ProgressManagerSettings};
 
 #[derive(Debug, Snafu)]
+#[allow(clippy::enum_variant_names)]
 pub(crate) enum SyncSettingsOperationError {
     #[snafu(display("failed to fetch current canister settings for canister {canister}"))]
     FetchCurrentSettings {

crates/icp-cli/tests/project_tests.rs

@@ -139,34 +139,132 @@ fn glob_path() {
         .success();
 }
 
-// TODO(or.ricon): This test is currently not passing, fix it.
-// #[test]
-// fn explicit_path_missing() {
-//     let ctx = TestContext::new();
-
-//     // Setup project
-//     let project_dir = ctx.create_project_dir("icp");
-
-//     // Project manifest
-//     let pm = r#"
-//     canisters:
-//       - my-canister
-//     "#;
-
-//     write_string(
-//         &project_dir.join("icp.yaml"), // path
-//         pm,                            // contents
-//     )
-//     .expect("failed to write project manifest");
-
-//     // Invoke build
-//     ctx.icp()
-//         .current_dir(project_dir)
-//         .args(["build"])
-//         .assert()
-//         .failure()
-//         .stderr(eq("Error: canister path must exist and be a directory \'my-canister\'").trim());
-// }
+#[test]
+fn explicit_path_missing() {
+    let ctx = TestContext::new();
+
+    // Setup project
+    let project_dir = ctx.create_project_dir("icp");
+
+    // Project manifest
+    let pm = r#"
+    canisters:
+      - my-canister
+    "#;
+
+    write_string(
+        &project_dir.join("icp.yaml"), // path
+        pm,                            // contents
+    )
+    .expect("failed to write project manifest");
+
+    // Invoke project show
+    ctx.icp()
+        .current_dir(project_dir)
+        .args(["project", "show"])
+        .assert()
+        .failure()
+        .stderr(contains(
+            "could not locate a canister manifest at: 'my-canister'",
+        ));
+}
+
+#[test]
+fn explicit_path_missing_canister_yaml() {
+    let ctx = TestContext::new();
+
+    // Setup project
+    let project_dir = ctx.create_project_dir("icp");
+
+    // Project manifest
+    let pm = r#"
+    canisters:
+      - my-canister
+    "#;
+
+    write_string(
+        &project_dir.join("icp.yaml"), // path
+        pm,                            // contents
+    )
+    .expect("failed to write project manifest");
+
+    // Create directory but no canister.yaml
+    create_dir_all(&project_dir.join("my-canister")).expect("failed to create canister directory");
+
+    // Invoke project show
+    ctx.icp()
+        .current_dir(project_dir)
+        .args(["project", "show"])
+        .assert()
+        .failure()
+        .stderr(contains(
+            "could not locate a canister manifest at: 'my-canister'",
+        ));
+}
+
+#[test]
+fn explicit_path_with_subdirectory() {
+    let ctx = TestContext::new();
+
+    // Setup project
+    let project_dir = ctx.create_project_dir("icp");
+
+    // Project manifest
+    let pm = r#"
+    canisters:
+      - canisters/backend
+      - canisters/frontend
+    "#;
+
+    write_string(
+        &project_dir.join("icp.yaml"), // path
+        pm,                            // contents
+    )
+    .expect("failed to write project manifest");
+
+    // Backend canister manifest
+    let backend_cm = indoc! {r#"
+        name: backend
+        build:
+          steps:
+            - type: script
+              command: echo "build"
+    "#};
+
+    create_dir_all(&project_dir.join("canisters/backend"))
+        .expect("failed to create backend directory");
+
+    write_string(
+        &project_dir.join("canisters/backend/canister.yaml"),
+        backend_cm,
+    )
+    .expect("failed to write backend manifest");
+
+    // Frontend canister manifest
+    let frontend_cm = indoc! {r#"
+        name: frontend
+        build:
+          steps:
+            - type: script
+              command: echo "build"
+    "#};
+
+    create_dir_all(&project_dir.join("canisters/frontend"))
+        .expect("failed to create frontend directory");
+
+    write_string(
+        &project_dir.join("canisters/frontend/canister.yaml"),
+        frontend_cm,
+    )
+    .expect("failed to write frontend manifest");
+
+    // Invoke project show - should succeed
+    ctx.icp()
+        .current_dir(&project_dir)
+        .args(["project", "show"])
+        .assert()
+        .success();
+}
 
 #[test]
 fn redefine_mainnet_network_disallowed() {
@@ -189,7 +287,7 @@ fn redefine_mainnet_network_disallowed() {
     // Any command that loads the project should fail
     ctx.icp()
         .current_dir(project_dir)
-        .args(["build"])
+        .args(["project", "show"])
         .assert()
         .failure()
         .stderr(contains("`mainnet` is a reserved network name"));

crates/icp/src/project.rs

@@ -143,7 +143,8 @@ pub async fn consolidate_manifest(
     for i in &m.canisters {
         let ms = match i {
             Item::Path(pattern) => {
-                let paths = match is_glob(pattern) {
+                let is_glob_pattern = is_glob(pattern);
+                let paths = match is_glob_pattern {
                     // Explicit path
                     false => vec![pdir.join(pattern)],
 
@@ -163,11 +164,28 @@ pub async fn consolidate_manifest(
                     }
                 };
 
-                let paths = paths
-                    .into_iter()
-                    .filter(|p| p.is_dir()) // Skip missing directories
-                    .filter(|p| p.join(CANISTER_MANIFEST).exists()) // Skip non-canister directories
-                    .collect::<Vec<_>>();
+                let paths = if is_glob_pattern {
+                    // For glob patterns, filter out non-directories and non-canister directories
+                    paths
+                        .into_iter()
+                        .filter(|p| p.is_dir())
+                        .filter(|p| p.join(CANISTER_MANIFEST).exists())
+                        .collect::<Vec<_>>()
+                } else {
+                    // For explicit paths, validate that they exist and contain canister.yaml
+                    let mut validated_paths = vec![];
+                    for p in paths {
+                        if !p.join(CANISTER_MANIFEST).is_file() {
+                            return NotFoundSnafu {
+                                kind: "canister".to_string(),
+                                path: pattern.to_string(),
+                            }
+                            .fail();
+                        }
+                        validated_paths.push(p);
+                    }
+                    validated_paths
+                };
 
                 let mut ms = vec![];