From 2e7cc4f1c5c90c8495d56e1e18d38c92f6142fed Mon Sep 17 00:00:00 2001 From: Raymond Khalife Date: Wed, 12 Aug 2026 04:48:52 +0000 Subject: [PATCH 1/3] fix: make `icp canister logs --json` emit JSON, not human text The two branches of `if args.json` in `fetch_and_display_logs` were swapped: `--json` printed the human-readable `format_log` lines while the default printed the `JsonListRecord` serialization. Swap them back so the non-follow path matches the follow path's orientation. Also terminate the JSON output with a newline so it does not leave the shell prompt mid-line. Co-Authored-By: Claude Opus 5 (1M context) --- crates/icp-cli/src/commands/canister/logs.rs | 24 ++--- crates/icp-cli/tests/canister_logs_tests.rs | 98 ++++++++++++++++++++ 2 files changed, 111 insertions(+), 11 deletions(-) diff --git a/crates/icp-cli/src/commands/canister/logs.rs b/crates/icp-cli/src/commands/canister/logs.rs index 78c66ac4e..6a70d47e7 100644 --- a/crates/icp-cli/src/commands/canister/logs.rs +++ b/crates/icp-cli/src/commands/canister/logs.rs @@ -1,4 +1,4 @@ -use std::io::stdout; +use std::io::{Write as _, stdout}; use anyhow::{Context as _, anyhow}; use candid::Principal; @@ -179,17 +179,9 @@ async fn fetch_and_display_logs( .context("Failed to fetch canister logs")?; if args.json { - println!( - "{}", - result - .canister_log_records - .iter() - .map(format_log) - .format("\n") - ); - } else { + let mut out = stdout(); serde_json::to_writer( - stdout(), + &mut out, &JsonListRecord { log_records: result .canister_log_records @@ -202,6 +194,16 @@ async fn fetch_and_display_logs( .collect(), }, )?; + writeln!(out)?; + } else { + println!( + "{}", + result + .canister_log_records + .iter() + .map(format_log) + .format("\n") + ); } Ok(()) diff --git a/crates/icp-cli/tests/canister_logs_tests.rs b/crates/icp-cli/tests/canister_logs_tests.rs index 49484f91a..44b699ec3 100644 --- a/crates/icp-cli/tests/canister_logs_tests.rs +++ b/crates/icp-cli/tests/canister_logs_tests.rs @@ -90,6 +90,104 @@ async fn canister_logs_single_fetch() { .stdout(contains("Test message 2")); } +/// Regression test: `--json` must emit machine-readable JSON and the default must emit the +/// human-readable formatted lines. These were once swapped. +#[cfg(unix)] // moc +#[tokio::test] +async fn canister_logs_single_fetch_json_orientation() { + let ctx = TestContext::new(); + let project_dir = ctx.create_project_dir("canister_logs"); + + ctx.copy_asset_dir("canister_logs", &project_dir); + + let pm = formatdoc! {r#" + canisters: + - name: logger + recipe: + type: "@dfinity/motoko@v4.0.0" + configuration: + main: main.mo + args: "" + + {NETWORK_RANDOM_PORT} + {ENVIRONMENT_RANDOM_PORT} + "#}; + + write_string(&project_dir.join("icp.yaml"), &pm).expect("failed to write project manifest"); + + let _g = ctx.start_network_in(&project_dir, "random-network").await; + ctx.ping_until_healthy(&project_dir, "random-network"); + + ctx.icp() + .current_dir(&project_dir) + .args(["deploy", "logger", "--environment", "random-environment"]) + .assert() + .success(); + + ctx.icp() + .current_dir(&project_dir) + .args([ + "canister", + "call", + "--environment", + "random-environment", + "logger", + "log", + "(\"Orientation message\")", + ]) + .assert() + .success(); + + // With --json: parseable JSON in the JsonListRecord shape, terminated by a newline. + let json_stdout = ctx + .icp() + .current_dir(&project_dir) + .args([ + "canister", + "logs", + "logger", + "--json", + "--environment", + "random-environment", + ]) + .assert() + .success() + .get_output() + .stdout + .clone(); + let json_stdout = String::from_utf8(json_stdout).expect("stdout is not valid UTF-8"); + assert!( + json_stdout.ends_with('\n'), + "--json output should end with a newline, got {json_stdout:?}" + ); + let parsed: serde_json::Value = + serde_json::from_str(json_stdout.trim()).expect("--json output should be valid JSON"); + let records = parsed["log_records"] + .as_array() + .expect("--json output should contain a log_records array"); + let record = records + .iter() + .find(|r| r["content"].as_str() == Some("Orientation message")) + .expect("--json output should contain the logged message"); + assert!(record["timestamp"].is_u64()); + assert!(record["index"].is_u64()); + + // Without --json: human-readable "[idx. timestamp]: content" lines, not JSON. + ctx.icp() + .current_dir(&project_dir) + .args([ + "canister", + "logs", + "logger", + "--environment", + "random-environment", + ]) + .assert() + .success() + .stdout(contains("]: Orientation message")) + .stdout(contains("log_records").not()); +} + #[cfg(unix)] // moc #[tokio::test] async fn canister_logs_follow_mode() { From 192f932f788b1bb7247547cc4731755cef86c7f1 Mon Sep 17 00:00:00 2001 From: Raymond Khalife Date: Wed, 12 Aug 2026 05:10:49 +0000 Subject: [PATCH 2/3] no-mistakes(review): add changelog entry, fold logs JSON test into existing test --- CHANGELOG.md | 2 + crates/icp-cli/tests/canister_logs_tests.rs | 86 ++++----------------- 2 files changed, 15 insertions(+), 73 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d9559443..f549b94e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ bump. Currently experimental: project bundling, project dependencies # Unreleased +* fix: `icp canister logs` no longer has its output formats swapped. `--json` emitted the human-readable lines and the default emitted JSON; now `--json` emits machine-readable JSON (terminated by a newline) and the default emits the human-readable lines. `--follow` was already correct and is unchanged. Scripts that relied on the inverted behavior — parsing the default output as JSON, or omitting `--json` to get JSON — must now pass `--json`. + # v1.3.0 * feat: a canister environment variable's value can now be read from a file, by writing `var: { path: }` in place of `var: value`. The path resolves against the canister's directory — including in an environment override, matching `init_args` — and surrounding whitespace is trimmed off the file's contents. The file is read when the project is loaded, so a missing file fails before anything is deployed. `icp project bundle` writes the value into the bundled manifest inline, rejecting a file outside the project as it does for other manifest file references. diff --git a/crates/icp-cli/tests/canister_logs_tests.rs b/crates/icp-cli/tests/canister_logs_tests.rs index 44b699ec3..c62a2c06b 100644 --- a/crates/icp-cli/tests/canister_logs_tests.rs +++ b/crates/icp-cli/tests/canister_logs_tests.rs @@ -74,7 +74,8 @@ async fn canister_logs_single_fetch() { .assert() .success(); - // Fetch logs + // Fetch logs: the default emits the human-readable "[idx. timestamp]: content" lines, + // not JSON. (Regression: this orientation was once swapped with `--json`.) ctx.icp() .current_dir(&project_dir) .args([ @@ -86,57 +87,9 @@ async fn canister_logs_single_fetch() { ]) .assert() .success() - .stdout(contains("Test message 1")) - .stdout(contains("Test message 2")); -} - -/// Regression test: `--json` must emit machine-readable JSON and the default must emit the -/// human-readable formatted lines. These were once swapped. -#[cfg(unix)] // moc -#[tokio::test] -async fn canister_logs_single_fetch_json_orientation() { - let ctx = TestContext::new(); - let project_dir = ctx.create_project_dir("canister_logs"); - - ctx.copy_asset_dir("canister_logs", &project_dir); - - let pm = formatdoc! {r#" - canisters: - - name: logger - recipe: - type: "@dfinity/motoko@v4.0.0" - configuration: - main: main.mo - args: "" - - {NETWORK_RANDOM_PORT} - {ENVIRONMENT_RANDOM_PORT} - "#}; - - write_string(&project_dir.join("icp.yaml"), &pm).expect("failed to write project manifest"); - - let _g = ctx.start_network_in(&project_dir, "random-network").await; - ctx.ping_until_healthy(&project_dir, "random-network"); - - ctx.icp() - .current_dir(&project_dir) - .args(["deploy", "logger", "--environment", "random-environment"]) - .assert() - .success(); - - ctx.icp() - .current_dir(&project_dir) - .args([ - "canister", - "call", - "--environment", - "random-environment", - "logger", - "log", - "(\"Orientation message\")", - ]) - .assert() - .success(); + .stdout(contains("]: Test message 1")) + .stdout(contains("]: Test message 2")) + .stdout(contains("log_records").not()); // With --json: parseable JSON in the JsonListRecord shape, terminated by a newline. let json_stdout = ctx @@ -165,27 +118,14 @@ async fn canister_logs_single_fetch_json_orientation() { let records = parsed["log_records"] .as_array() .expect("--json output should contain a log_records array"); - let record = records - .iter() - .find(|r| r["content"].as_str() == Some("Orientation message")) - .expect("--json output should contain the logged message"); - assert!(record["timestamp"].is_u64()); - assert!(record["index"].is_u64()); - - // Without --json: human-readable "[idx. timestamp]: content" lines, not JSON. - ctx.icp() - .current_dir(&project_dir) - .args([ - "canister", - "logs", - "logger", - "--environment", - "random-environment", - ]) - .assert() - .success() - .stdout(contains("]: Orientation message")) - .stdout(contains("log_records").not()); + for message in ["Test message 1", "Test message 2"] { + let record = records + .iter() + .find(|r| r["content"].as_str() == Some(message)) + .unwrap_or_else(|| panic!("--json output should contain {message:?}")); + assert!(record["timestamp"].is_u64()); + assert!(record["index"].is_u64()); + } } #[cfg(unix)] // moc From b4c32d172c1b1e02ac9ac9467f20ba2fd1d05dc0 Mon Sep 17 00:00:00 2001 From: Raymond Khalife Date: Wed, 12 Aug 2026 05:27:00 +0000 Subject: [PATCH 3/3] no-mistakes(document): document --json newline rationale in canister logs --- crates/icp-cli/src/commands/canister/logs.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/icp-cli/src/commands/canister/logs.rs b/crates/icp-cli/src/commands/canister/logs.rs index 6a70d47e7..31cae22c8 100644 --- a/crates/icp-cli/src/commands/canister/logs.rs +++ b/crates/icp-cli/src/commands/canister/logs.rs @@ -194,6 +194,9 @@ async fn fetch_and_display_logs( .collect(), }, )?; + // Unlike the other `--json` sites in this module, terminate the document with a + // newline: it is the entirety of stdout, so without it the shell prompt lands + // mid-line. Covered by `canister_logs_single_fetch`. writeln!(out)?; } else { println!(