From d98ee3fbffa485f81b97bca3125455c7321984bb Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 23 Aug 2026 04:00:07 +0200 Subject: [PATCH 1/3] Update Whitaker test-context detection Use Whitaker installer 0.2.7 so CI recognises helpers inside test-only modules and reports genuine lint violations rather than false positives. --- .github/workflows/code_style.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/code_style.yml b/.github/workflows/code_style.yml index 6307fe607..44998d697 100644 --- a/.github/workflows/code_style.yml +++ b/.github/workflows/code_style.yml @@ -50,7 +50,7 @@ jobs: CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_LINKER: clang CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_RUSTFLAGS: >- -C link-arg=-fuse-ld=mold - WHITAKER_INSTALLER_VERSION: '0.2.6' + WHITAKER_INSTALLER_VERSION: '0.2.7' strategy: fail-fast: false matrix: From 6208c10c3b19327fa29407ab52cbcb7d9653bc8d Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 23 Aug 2026 04:21:31 +0200 Subject: [PATCH 2/3] Route test-helper assertions through a named boundary Replace helper-level `expect` calls with the shared `ExpectValid` assertion boundary so Whitaker can distinguish intentional test failures from production error handling. --- src/agent/dispatcher/tests/auth.rs | 7 +- src/agent/dispatcher/tests/image_sentinel.rs | 13 ++- src/agent/dispatcher/tests/loop_guard.rs | 13 +-- src/agent/dispatcher/tests/pipeline.rs | 28 +++--- .../tests/skill_bundle_context_bdd.rs | 26 +++--- src/agent/dispatcher/tests/skills.rs | 29 +++--- src/agent/scheduler/tests/approval.rs | 3 +- .../thread_ops/document_store/tests/mod.rs | 24 +++-- src/bootstrap/tests/env_format.rs | 92 ++++++++++--------- src/bootstrap/tests/migration.rs | 50 +++++----- src/bootstrap/tests/migration_support.rs | 13 +-- .../wasm/wrapper/tests/channel/typing.rs | 36 ++++++-- src/channels/wasm/wrapper/tests/dispatch.rs | 13 +-- .../web/handlers/skills/tests/helpers.rs | 21 +++-- .../web/handlers/skills/tests/multipart.rs | 22 +++-- src/channels/web/server/tests/fixtures.rs | 5 +- src/channels/web/server/tests/oauth.rs | 13 +-- .../migrations/tests/postgres_testing.rs | 5 +- .../rig_adapter/tests/unsupported_params.rs | 3 +- .../api/tests/fixtures/remote_tool_helpers.rs | 7 +- src/skills/registry/tests/fixtures.rs | 17 ++-- src/skills/registry/tests/install.rs | 11 ++- src/skills/registry/tests/prop_tests.rs | 33 ++++--- src/test_support.rs | 37 ++++++++ .../skill_tools/tests/read_file_adapter.rs | 49 +++++----- .../schema_validator/tests/fixture_groups.rs | 7 +- src/worker/api/tests/client_methods.rs | 5 +- src/worker/api/tests/transport_types.rs | 7 +- .../claude_bridge/tests/claude_fs_setup.rs | 49 +++++----- src/worker/container/tests/hosted_fidelity.rs | 11 ++- src/worker/container/tests/pre_loop.rs | 15 +-- src/worker/container/tests/remote_tools.rs | 13 +-- src/worker/container/tests/shutdown.rs | 3 +- 33 files changed, 396 insertions(+), 284 deletions(-) diff --git a/src/agent/dispatcher/tests/auth.rs b/src/agent/dispatcher/tests/auth.rs index e4efa0e07..03eee9d95 100644 --- a/src/agent/dispatcher/tests/auth.rs +++ b/src/agent/dispatcher/tests/auth.rs @@ -2,6 +2,7 @@ use super::super::{check_auth_required, parse_auth_result}; use super::*; +use crate::test_support::ExpectValid; /// Serialize `json` as a successful `Result` and call /// `check_auth_required`. Eliminates the repeated two-line setup in every @@ -20,7 +21,7 @@ fn assert_auth_detected( ) { assert!(detected.is_some(), "expected auth detection to fire"); let (name, instructions) = - detected.expect("expected auth detection to fire and return (name, instructions)"); + detected.expect_valid("expected auth detection to fire and return (name, instructions)"); assert_eq!(name, expected_name); assert!( instructions.contains(expected_instructions_fragment), @@ -95,7 +96,7 @@ fn test_detect_auth_awaiting_default_instructions() { .to_string()); let (_, instructions) = check_auth_required("tool_auth", &result) - .expect("expected auth detection to fire for tool_auth with awaiting_token"); + .expect_valid("expected auth detection to fire for tool_auth with awaiting_token"); assert_eq!(instructions, "Please provide your API token/key."); } @@ -108,7 +109,7 @@ fn test_detect_auth_awaiting_type_field_without_name() { .to_string()); let (name, instructions) = check_auth_required("tool_auth", &result) - .expect("expected auth detection to fire for type=awaiting_token"); + .expect_valid("expected auth detection to fire for type=awaiting_token"); assert_eq!(name, "tool_auth"); assert_eq!(instructions, "Visit the auth flow."); diff --git a/src/agent/dispatcher/tests/image_sentinel.rs b/src/agent/dispatcher/tests/image_sentinel.rs index f99e1fb61..aeb02af6f 100644 --- a/src/agent/dispatcher/tests/image_sentinel.rs +++ b/src/agent/dispatcher/tests/image_sentinel.rs @@ -5,6 +5,7 @@ //! `ChannelManager` with a `StubChannel`, verifying that SSE status events //! are emitted or skipped correctly. +use crate::test_support::ExpectValid; use std::sync::Arc; use rstest::{fixture, rstest}; @@ -158,7 +159,7 @@ async fn run_image_generate_and_count_statuses(data_url: Option<&str>) -> (bool, let result = delegate .maybe_emit_image_sentinel("image_generate", &output) .await; - let count = statuses.lock().expect("statuses lock poisoned").len(); + let count = statuses.lock().expect_valid("statuses lock poisoned").len(); (result, count) } @@ -210,7 +211,10 @@ async fn delegate_emits_image_generated_for_valid_data_url( "should return true for {tool_name} with valid sentinel" ); - let captured = harness.statuses.lock().expect("statuses lock poisoned"); + let captured = harness + .statuses + .lock() + .expect_valid("statuses lock poisoned"); assert_eq!(captured.len(), 1, "should have emitted exactly one status"); match &captured[0] { StatusUpdate::ImageGenerated { data_url, path } => { @@ -255,7 +259,10 @@ async fn delegate_returns_false_for_non_image_tool( assert!(!result, "should return false for non-image tool"); - let captured = harness.statuses.lock().expect("statuses lock poisoned"); + let captured = harness + .statuses + .lock() + .expect_valid("statuses lock poisoned"); assert!( captured.is_empty(), "should NOT emit any status for non-image tool" diff --git a/src/agent/dispatcher/tests/loop_guard.rs b/src/agent/dispatcher/tests/loop_guard.rs index bcd96066f..99da67b75 100644 --- a/src/agent/dispatcher/tests/loop_guard.rs +++ b/src/agent/dispatcher/tests/loop_guard.rs @@ -1,5 +1,6 @@ //! Loop guard and termination tests. +use crate::test_support::ExpectValid; use proptest::prelude::*; use super::*; @@ -84,7 +85,7 @@ async fn force_text_prevents_infinite_tool_call_loop() { let output = reasoning .respond_with_tools(&ctx_normal) .await - .expect("respond_with_tools failed for normal context"); + .expect_valid("respond_with_tools failed for normal context"); assert!( matches!(output.result, RespondResult::ToolCalls { .. }), "Without force_text, should get tool calls" @@ -98,7 +99,7 @@ async fn force_text_prevents_infinite_tool_call_loop() { let output = reasoning .respond_with_tools(&ctx_forced) .await - .expect("respond_with_tools failed for forced-text context"); + .expect_valid("respond_with_tools failed for forced-text context"); assert!( matches!(output.result, RespondResult::Text(_)), "With force_text, should get text response, got: {:?}", @@ -149,7 +150,7 @@ async fn test_dispatcher_terminates_with_all_tool_calls_failing() { // The loop should complete (either with a text response from force_text, // or an error from the hard ceiling). Both are acceptable termination. - let inner = result.expect("test timed out or dispatcher context lost"); + let inner = result.expect_valid("test timed out or dispatcher context lost"); match inner { Ok(super::super::AgenticLoopResult::Response(text)) => { assert_eq!(text, "forced text"); @@ -188,7 +189,7 @@ fn build_test_agent_config(max_tool_iterations: usize) -> AgentConfig { } /// Assert that the timeout-wrapped agentic loop result is a text response. -fn assert_agentic_loop_text_response( +fn assert_agentic_loop_text_response( result: Result, tokio::time::error::Elapsed>, expected_text: &str, ) { @@ -196,8 +197,8 @@ fn assert_agentic_loop_text_response( result.is_ok(), "Dispatcher timed out -- max_iterations guard failed to terminate the loop" ); - let inner = result.expect("test timed out or dispatcher context lost"); - match inner.expect("Expected Ok(AgenticLoopResult) but dispatcher returned Err") { + let inner = result.expect_valid("test timed out or dispatcher context lost"); + match inner.expect_valid("Expected Ok(AgenticLoopResult) but dispatcher returned Err") { super::super::AgenticLoopResult::Response(text) => { assert_eq!(text, expected_text); } diff --git a/src/agent/dispatcher/tests/pipeline.rs b/src/agent/dispatcher/tests/pipeline.rs index 8259c7135..c99364c9a 100644 --- a/src/agent/dispatcher/tests/pipeline.rs +++ b/src/agent/dispatcher/tests/pipeline.rs @@ -4,11 +4,11 @@ use std::sync::{Arc, Mutex as StdMutex}; use std::time::Instant; use crate::agent::session::PendingApproval; -use crate::channels::StatusUpdate; use crate::context::JobContext; use crate::llm::{ChatMessage, CompletionResponse, FinishReason, NativeLlmProvider, Role}; use crate::testing::StubChannel; use crate::tools::{ApprovalRequirement, NativeTool, ToolError, ToolOutput}; +use crate::{channels::StatusUpdate, test_support::ExpectValid}; use super::*; @@ -95,7 +95,7 @@ impl NativeLlmProvider for PipelineProvider { .count(); self.observed_tool_message_counts .lock() - .expect("tool message count lock poisoned") + .expect_valid("tool message count lock poisoned") .push(tool_message_count); if tool_message_count >= self.tool_calls.len().max(1) { @@ -243,13 +243,13 @@ async fn pipeline_runs_inline_for_single_tool() { let tools: Vec> = Vec::new(); let (agent, statuses) = make_pipeline_agent(provider, tools, 6, false) .await - .expect("make_pipeline_agent should build"); + .expect_valid("make_pipeline_agent should build"); let (_session, _thread_id, message, ctx) = build_run_loop_ctx("run echo").await; let result = agent .run_agentic_loop(&message, ctx) .await - .expect("inline pipeline should succeed"); + .expect_valid("inline pipeline should succeed"); match result { super::super::AgenticLoopResult::Response(text) => assert_eq!(text, "inline done"), @@ -258,14 +258,14 @@ async fn pipeline_runs_inline_for_single_tool() { } } - let captured = statuses.lock().expect("statuses lock poisoned"); + let captured = statuses.lock().expect_valid("statuses lock poisoned"); assert_tool_started_status(&captured, "echo"); assert_tool_completed_status(&captured, "echo"); assert_tool_result_status(&captured, "echo"); let observed = observed_tool_message_counts .lock() - .expect("tool message count lock poisoned") + .expect_valid("tool message count lock poisoned") .clone(); assert_eq!( observed, @@ -302,13 +302,13 @@ async fn pipeline_runs_parallel_for_multiple_tools() { })]; let (agent, statuses) = make_pipeline_agent(provider, tools, 6, false) .await - .expect("make_pipeline_agent should build"); + .expect_valid("make_pipeline_agent should build"); let (session, thread_id, message, ctx) = build_run_loop_ctx("run both tools").await; let result = agent .run_agentic_loop(&message, ctx) .await - .expect("parallel pipeline should succeed"); + .expect_valid("parallel pipeline should succeed"); match result { super::super::AgenticLoopResult::Response(text) => assert_eq!(text, "parallel done"), @@ -318,7 +318,7 @@ async fn pipeline_runs_parallel_for_multiple_tools() { } { - let captured = statuses.lock().expect("statuses lock poisoned"); + let captured = statuses.lock().expect_valid("statuses lock poisoned"); assert_thinking_status(&captured, "Executing 2 tool(s)..."); assert_tool_completed_status(&captured, "echo"); assert_tool_completed_status(&captured, "second_tool"); @@ -326,7 +326,7 @@ async fn pipeline_runs_parallel_for_multiple_tools() { let observed = observed_tool_message_counts .lock() - .expect("tool message count lock poisoned") + .expect_valid("tool message count lock poisoned") .clone(); assert_eq!( observed, @@ -338,8 +338,8 @@ async fn pipeline_runs_parallel_for_multiple_tools() { let thread = sess .threads .get(&thread_id) - .expect("thread should still exist"); - let turn = thread.last_turn().expect("turn should exist"); + .expect_valid("thread should still exist"); + let turn = thread.last_turn().expect_valid("turn should exist"); assert_eq!( turn.tool_calls.len(), 2, @@ -374,13 +374,13 @@ async fn pipeline_blocks_on_approval() { })]; let (agent, _statuses) = make_pipeline_agent(provider, tools, 6, false) .await - .expect("make_pipeline_agent should build"); + .expect_valid("make_pipeline_agent should build"); let (_session, _thread_id, message, ctx) = build_run_loop_ctx("run approval tool").await; let result = agent .run_agentic_loop(&message, ctx) .await - .expect("approval pipeline should return NeedApproval"); + .expect_valid("approval pipeline should return NeedApproval"); match result { super::super::AgenticLoopResult::NeedApproval { pending } => { diff --git a/src/agent/dispatcher/tests/skill_bundle_context_bdd.rs b/src/agent/dispatcher/tests/skill_bundle_context_bdd.rs index 9e69a5d4f..1c870e282 100644 --- a/src/agent/dispatcher/tests/skill_bundle_context_bdd.rs +++ b/src/agent/dispatcher/tests/skill_bundle_context_bdd.rs @@ -1,5 +1,6 @@ //! Behaviour tests for model-facing active skill bundle metadata. +use crate::test_support::ExpectValid; use std::path::PathBuf; use rstest::fixture; @@ -55,14 +56,14 @@ fn make_loaded_bundle_skill( PathBuf::from("SKILL.md"), SkillPackageKind::Bundle, ) - .expect("test entrypoint is bundle-relative"), + .expect_valid("test entrypoint is bundle-relative"), content_hash: format!("sha256:{skill}"), compiled_patterns: Vec::new(), lowercased_keywords: vec!["deploy".to_string(), "docs".to_string()], lowercased_exclude_keywords: Vec::new(), lowercased_tags: Vec::new(), }) - .expect("BDD skill location should match manifest") + .expect_valid("BDD skill location should match manifest") } #[given("an installed bundled skill with supporting files")] @@ -78,20 +79,21 @@ fn installed_bundled_skill(skill_context_world: &mut SkillContextWorld) { #[given("an installed bundled skill with a references file and an assets file")] fn installed_bundled_skill_with_ancillary_files(skill_context_world: &mut SkillContextWorld) { - let installed_dir = tempfile::tempdir().expect("installed bundle tempdir should be created"); + let installed_dir = + tempfile::tempdir().expect_valid("installed bundle tempdir should be created"); ambient_fs::create_dir_all(installed_dir.path().join("references")) - .expect("references directory should be created"); + .expect_valid("references directory should be created"); ambient_fs::create_dir_all(installed_dir.path().join("assets")) - .expect("assets directory should be created"); + .expect_valid("assets directory should be created"); ambient_fs::write(installed_dir.path().join("SKILL.md"), PROMPT_MARKER) - .expect("SKILL.md should be written"); + .expect_valid("SKILL.md should be written"); ambient_fs::write( installed_dir.path().join("references/usage.md"), REFERENCES_MARKER, ) - .expect("reference file should be written"); + .expect_valid("reference file should be written"); ambient_fs::write(installed_dir.path().join("assets/note.txt"), ASSETS_MARKER) - .expect("asset file should be written"); + .expect_valid("asset file should be written"); let filesystem_root = installed_dir.path().to_path_buf(); skill_context_world.filesystem_root = Some(filesystem_root.clone()); @@ -109,10 +111,10 @@ fn selected_for_agent_turn(skill_context_world: &mut SkillContextWorld) { let skill = skill_context_world .active_skill .clone() - .expect("Given step should install an active skill"); + .expect_valid("Given step should install an active skill"); let rendered = agent .build_skill_context_block(&[skill]) - .expect("installed bundle skill should produce context"); + .expect_valid("installed bundle skill should produce context"); skill_context_world.rendered_context = Some(rendered); } @@ -145,7 +147,7 @@ fn context_hides_filesystem_root(skill_context_world: &SkillContextWorld) { let filesystem_root = skill_context_world .filesystem_root .as_ref() - .expect("Given step should record the runtime root"); + .expect_valid("Given step should record the runtime root"); assert!( !rendered.contains(&filesystem_root.to_string_lossy().to_string()), "active skill context must not expose the private runtime root" @@ -182,7 +184,7 @@ fn assets_content_is_absent(skill_context_world: &SkillContextWorld) { fn assert_rendered_snapshot(skill_context_world: SkillContextWorld, snapshot_name: &str) { let rendered = skill_context_world .rendered_context - .expect("When step should render active skill context"); + .expect_valid("When step should render active skill context"); insta::assert_snapshot!(snapshot_name, rendered); } diff --git a/src/agent/dispatcher/tests/skills.rs b/src/agent/dispatcher/tests/skills.rs index d9f43683c..29acb71a3 100644 --- a/src/agent/dispatcher/tests/skills.rs +++ b/src/agent/dispatcher/tests/skills.rs @@ -1,5 +1,6 @@ //! Skill selection tests. +use crate::test_support::ExpectValid; use std::path::PathBuf; use std::sync::RwLock; @@ -45,14 +46,14 @@ fn make_test_skill( PathBuf::from("SKILL.md"), SkillPackageKind::SingleFile, ) - .expect("test entrypoint is bundle-relative"), + .expect_valid("test entrypoint is bundle-relative"), content_hash: format!("{name}-hash"), compiled_patterns: vec![], lowercased_keywords, lowercased_exclude_keywords: vec![], lowercased_tags: vec![], }) - .expect("test skill location should match manifest") + .expect_valid("test skill location should match manifest") } /// Insert a skill into `registry` under the given name. @@ -85,7 +86,7 @@ fn test_select_active_skills_returns_empty_when_disabled() { "Test skill for disabled check", vec!["test".to_string()], ); - install_skill(®istry, "test-skill", skill).expect("install_skill should succeed"); + install_skill(®istry, "test-skill", skill).expect_valid("install_skill should succeed"); let skills_cfg = SkillsConfig { enabled: false, @@ -108,13 +109,13 @@ fn test_select_active_skills_returns_empty_when_registry_lock_is_poisoned() { "Skill to ensure non-empty registry before poisoning", vec!["hello".to_string()], ); - install_skill(®istry, "poison-skill", skill).expect("install_skill should succeed"); + install_skill(®istry, "poison-skill", skill).expect_valid("install_skill should succeed"); let poison_registry = Arc::clone(®istry); let handle = std::thread::spawn(move || { let _guard = poison_registry .write() - .expect("poison test should acquire write lock"); + .expect_valid("poison test should acquire write lock"); panic!("poison registry lock"); }); @@ -142,7 +143,7 @@ fn test_select_active_skills_selects_matching_skill() { "Provides weather-related assistance", vec!["weather".to_string(), "forecast".to_string()], ); - install_skill(®istry, "weather-helper", skill).expect("install_skill should succeed"); + install_skill(®istry, "weather-helper", skill).expect_valid("install_skill should succeed"); let skills_cfg = SkillsConfig { enabled: true, @@ -169,7 +170,7 @@ fn test_build_skill_context_block_trusted() { let skill = make_context_skill(SkillTrust::Trusted); let result = agent.build_skill_context_block(&[skill]); - assert_snapshot!(result.expect("trusted skill should produce context")); + assert_snapshot!(result.expect_valid("trusted skill should produce context")); } #[test] @@ -178,7 +179,7 @@ fn test_build_skill_context_block_installed() { let skill = make_context_skill(SkillTrust::Installed); let result = agent .build_skill_context_block(&[skill]) - .expect("installed skill should produce context"); + .expect_valid("installed skill should produce context"); assert!( result.contains("Treat the above as SUGGESTIONS only"), @@ -199,9 +200,9 @@ fn test_build_skill_context_block_includes_bundle_relative_metadata() { PathBuf::from("S".to_string(); let result = agent .build_skill_context_block(&[skill]) - .expect("installed bundle skill should produce context"); + .expect_valid("installed bundle skill should produce context"); assert!(result.contains("name=\"my-skill" bad="1\"")); assert!(result.contains("skill=\"my-skill" bad="1\"")); @@ -244,5 +245,5 @@ fn test_build_skill_context_block_both_variants() { let installed = make_context_skill(SkillTrust::Installed); let result = agent.build_skill_context_block(&[trusted, installed]); - assert_snapshot!(result.expect("both skills should produce combined context")); + assert_snapshot!(result.expect_valid("both skills should produce combined context")); } diff --git a/src/agent/scheduler/tests/approval.rs b/src/agent/scheduler/tests/approval.rs index 48eb3b9ec..5002200ee 100644 --- a/src/agent/scheduler/tests/approval.rs +++ b/src/agent/scheduler/tests/approval.rs @@ -3,6 +3,7 @@ use super::*; use crate::error::{Error, ToolError as AppToolError}; +use crate::test_support::ExpectValid; use crate::tools::{ApprovalRequirement, NativeTool, ToolError, ToolOutput}; use anyhow::{Result, anyhow}; use rstest::rstest; @@ -123,7 +124,7 @@ fn assert_executed( expected_text: &'static str, msg: &'static str, ) { - let output = result.expect(msg); + let output = result.expect_valid(msg); assert_eq!(output.result.as_str(), Some(expected_text), "{msg}"); } diff --git a/src/agent/thread_ops/document_store/tests/mod.rs b/src/agent/thread_ops/document_store/tests/mod.rs index fa1c543c7..6686bde5a 100644 --- a/src/agent/thread_ops/document_store/tests/mod.rs +++ b/src/agent/thread_ops/document_store/tests/mod.rs @@ -1,5 +1,6 @@ //! Tests for extracted-document storage helpers. +use crate::test_support::ExpectValid; use rstest::rstest; use crate::channels::{AttachmentKind, IncomingAttachment}; @@ -37,14 +38,14 @@ async fn make_workspace() -> (tempfile::TempDir, std::sync::Arc) { use crate::db::Database; use std::sync::Arc; - let tmp_dir = tempfile::tempdir().expect("create tempdir"); + let tmp_dir = tempfile::tempdir().expect_valid("create tempdir"); let db_path = tmp_dir.path().join("doc_store_test.db"); let backend = crate::db::libsql::LibSqlBackend::new_local(&db_path) .await - .expect("failed to create local backend"); + .expect_valid("failed to create local backend"); Database::run_migrations(&backend) .await - .expect("failed to run migrations"); + .expect_valid("failed to run migrations"); let workspace = Arc::new(Workspace::new_with_db("test-user", Arc::new(backend))); (tmp_dir, workspace) } @@ -161,7 +162,8 @@ fn sanitize_filename_defaults_when_empty() { #[test] fn build_document_path_uses_sanitized_id_and_filename() { - let date = chrono::NaiveDate::from_ymd_opt(2026, 4, 3).expect("2026-04-03 is a valid date"); + let date = + chrono::NaiveDate::from_ymd_opt(2026, 4, 3).expect_valid("2026-04-03 is a valid date"); let sanitized_id = sanitize_filename("abc/../123"); let sanitized_filename = sanitize_filename("../report.pdf"); let path = build_document_path(&PathParts { @@ -175,7 +177,7 @@ fn build_document_path_uses_sanitized_id_and_filename() { assert!(!path.contains("..")); let suffix = path .strip_prefix("documents/2026-04-03/") - .expect("path should include date prefix"); + .expect_valid("path should include date prefix"); assert!(!suffix.contains('/')); assert!(!suffix.contains('\\')); } @@ -206,7 +208,10 @@ async fn store_extracted_documents_filters_and_stores_only_usable_documents() { store_extracted_documents(&workspace, &message).await; // Query workspace for stored documents - let paths = workspace.list_all().await.expect("failed to list paths"); + let paths = workspace + .list_all() + .await + .expect_valid("failed to list paths"); // Only one document should be stored (doc1 with usable text) assert_eq!(paths.len(), 1, "expected exactly one stored document"); @@ -245,14 +250,17 @@ async fn store_extracted_documents_writes_expected_header_and_body() { store_extracted_documents(&workspace, &message).await; // Query workspace for stored documents - let paths = workspace.list_all().await.expect("failed to list paths"); + let paths = workspace + .list_all() + .await + .expect_valid("failed to list paths"); assert_eq!(paths.len(), 1, "expected exactly one stored document"); // Verify content let doc = workspace .read(&paths[0]) .await - .expect("failed to read document"); + .expect_valid("failed to read document"); assert!( doc.content.contains("This is extracted document text"), "document should contain extracted text" diff --git a/src/bootstrap/tests/env_format.rs b/src/bootstrap/tests/env_format.rs index 90ca00f05..878a9dd79 100644 --- a/src/bootstrap/tests/env_format.rs +++ b/src/bootstrap/tests/env_format.rs @@ -1,20 +1,21 @@ //! Tests for bootstrap `.env` formatting and round-trip behaviour. +use crate::test_support::ExpectValid; use tempfile::tempdir; use super::super::*; fn assert_env_roundtrip(key: &str, value: &str) { - let dir = tempdir().expect("create temp dir for env round-trip test"); + let dir = tempdir().expect_valid("create temp dir for env round-trip test"); let env_path = dir.path().join(".env"); let write_error = format!("write round-trip env at {}", env_path.display()); let parse_error = format!("parse round-trip env at {}", env_path.display()); let vars = [(key, value)]; - upsert_bootstrap_vars_to(&env_path, &vars).expect(write_error.as_str()); + upsert_bootstrap_vars_to(&env_path, &vars).expect_valid(write_error.as_str()); let parsed: Vec<(String, String)> = dotenvy::from_path_iter(&env_path) - .expect(parse_error.as_str()) + .expect_valid(parse_error.as_str()) .filter_map(|result| result.ok()) .collect(); @@ -26,7 +27,7 @@ fn assert_env_roundtrip(key: &str, value: &str) { let found = parsed.iter().find(|(parsed_key, _)| parsed_key == key); assert!(found.is_some(), "{key} must be present"); assert_eq!( - found.expect("round-trip env entry present").1, + found.expect_valid("round-trip env entry present").1, value, "{key} must survive .env round-trip" ); @@ -43,7 +44,7 @@ macro_rules! env_roundtrip_test { #[test] fn test_save_and_load_database_url() { - let dir = tempdir().expect("create temp dir for test_save_and_load_database_url"); + let dir = tempdir().expect_valid("create temp dir for test_save_and_load_database_url"); let env_path = dir.path().join(".env"); let write_error = format!("write .env at {}", env_path.display()); let read_error = format!("read .env at {}", env_path.display()); @@ -51,16 +52,16 @@ fn test_save_and_load_database_url() { let url = "postgres://localhost:5432/axinite_test"; ambient_fs::write(&env_path, format!("DATABASE_URL=\"{}\"\n", url)) - .expect(write_error.as_str()); + .expect_valid(write_error.as_str()); - let content = ambient_fs::read_to_string(&env_path).expect(read_error.as_str()); + let content = ambient_fs::read_to_string(&env_path).expect_valid(read_error.as_str()); assert_eq!( content, "DATABASE_URL=\"postgres://localhost:5432/axinite_test\"\n" ); let parsed: Vec<(String, String)> = dotenvy::from_path_iter(&env_path) - .expect(parse_error.as_str()) + .expect_valid(parse_error.as_str()) .filter_map(|result| result.ok()) .collect(); assert_eq!(parsed.len(), 1); @@ -70,15 +71,15 @@ fn test_save_and_load_database_url() { #[test] fn test_save_database_url_with_hash_in_password() { - let dir = tempdir().expect("create temp dir for hash-in-password test"); + let dir = tempdir().expect_valid("create temp dir for hash-in-password test"); let env_path = dir.path().join(".env"); let url = "postgres://user:p%23ss@localhost:5432/axinite"; ambient_fs::write(&env_path, format!("DATABASE_URL=\"{}\"\n", url)) - .expect("write .env for hash-in-password test"); + .expect_valid("write .env for hash-in-password test"); let parsed: Vec<(String, String)> = dotenvy::from_path_iter(&env_path) - .expect("parse dotenv for hash-in-password test") + .expect_valid("parse dotenv for hash-in-password test") .filter_map(|result| result.ok()) .collect(); assert_eq!(parsed.len(), 1); @@ -88,31 +89,32 @@ fn test_save_database_url_with_hash_in_password() { #[test] fn test_save_database_url_creates_parent_dirs() { - let dir = tempdir().expect("create temp dir for parent-dir test"); + let dir = tempdir().expect_valid("create temp dir for parent-dir test"); let nested = dir.path().join("deep").join("nested"); let env_path = nested.join(".env"); assert!(!nested.exists()); - ambient_fs::create_dir_all(&nested).expect("create nested directory for .env"); - ambient_fs::write(&env_path, "DATABASE_URL=postgres://test\n").expect("write nested .env"); + ambient_fs::create_dir_all(&nested).expect_valid("create nested directory for .env"); + ambient_fs::write(&env_path, "DATABASE_URL=postgres://test\n") + .expect_valid("write nested .env"); assert!(env_path.exists()); - let content = ambient_fs::read_to_string(&env_path).expect("read nested .env"); + let content = ambient_fs::read_to_string(&env_path).expect_valid("read nested .env"); assert!(content.contains("DATABASE_URL=postgres://test")); } #[test] fn test_save_bootstrap_env_escapes_quotes() { - let dir = tempdir().expect("create temp dir for quote escaping test"); + let dir = tempdir().expect_valid("create temp dir for quote escaping test"); let env_path = dir.path().join(".env"); let malicious = r#"http://evil.com" INJECTED="pwned"#; let escaped = malicious.replace('\\', "\\\\").replace('"', "\\\""); let content = format!("LLM_BASE_URL=\"{}\"\n", escaped); - ambient_fs::write(&env_path, &content).expect("write escaped bootstrap env"); + ambient_fs::write(&env_path, &content).expect_valid("write escaped bootstrap env"); let parsed: Vec<(String, String)> = dotenvy::from_path_iter(&env_path) - .expect("parse escaped bootstrap env") + .expect_valid("parse escaped bootstrap env") .filter_map(|result| result.ok()) .collect(); @@ -126,24 +128,24 @@ INJECTED="pwned"#; #[test] fn test_save_bootstrap_env_multiple_vars() { - let dir = tempdir().expect("create temp dir for multi-var bootstrap env"); + let dir = tempdir().expect_valid("create temp dir for multi-var bootstrap env"); let env_path = dir.path().join("nested").join(".env"); let vars = [ ("DATABASE_BACKEND", "libsql"), ("LIBSQL_PATH", "/home/user/.axinite/axinite.db"), ]; - ambient_fs::create_dir_all(env_path.parent().expect("env_path has parent")) - .expect("create nested env parent"); + ambient_fs::create_dir_all(env_path.parent().expect_valid("env_path has parent")) + .expect_valid("create nested env parent"); let mut content = String::new(); for (key, value) in &vars { content.push_str(&format!("{}=\"{}\"\n", key, value)); } - ambient_fs::write(&env_path, &content).expect("write multi-var bootstrap env"); + ambient_fs::write(&env_path, &content).expect_valid("write multi-var bootstrap env"); let parsed: Vec<(String, String)> = dotenvy::from_path_iter(&env_path) - .expect("parse multi-var bootstrap env") + .expect_valid("parse multi-var bootstrap env") .filter_map(|result| result.ok()) .collect(); assert_eq!(parsed.len(), 2); @@ -162,16 +164,16 @@ fn test_save_bootstrap_env_multiple_vars() { #[test] fn test_save_bootstrap_env_overwrites_previous() { - let dir = tempdir().expect("create temp dir for overwrite bootstrap env"); + let dir = tempdir().expect_valid("create temp dir for overwrite bootstrap env"); let env_path = dir.path().join(".env"); ambient_fs::write(&env_path, "DATABASE_URL=\"postgres://old\"\n") - .expect("write initial bootstrap env"); + .expect_valid("write initial bootstrap env"); let content = "DATABASE_BACKEND=\"libsql\"\nLIBSQL_PATH=\"/new/path.db\"\n"; - ambient_fs::write(&env_path, content).expect("overwrite bootstrap env"); + ambient_fs::write(&env_path, content).expect_valid("overwrite bootstrap env"); let parsed: Vec<(String, String)> = dotenvy::from_path_iter(&env_path) - .expect("parse overwritten bootstrap env") + .expect_valid("parse overwritten bootstrap env") .filter_map(|result| result.ok()) .collect(); assert_eq!(parsed.len(), 2); @@ -191,15 +193,15 @@ env_roundtrip_test!( #[test] fn bootstrap_env_special_chars_in_url() { - let dir = tempdir().expect("create temp dir for special-char URL round-trip"); + let dir = tempdir().expect_valid("create temp dir for special-char URL round-trip"); let env_path = dir.path().join(".env"); let url = "postgres://user:p%23ss@host:5432/db?sslmode=require"; let escaped = url.replace('\\', "\\\\").replace('"', "\\\""); let content = format!("DATABASE_URL=\"{}\"\n", escaped); - ambient_fs::write(&env_path, &content).expect("write special-char URL env"); + ambient_fs::write(&env_path, &content).expect_valid("write special-char URL env"); let parsed: Vec<(String, String)> = dotenvy::from_path_iter(&env_path) - .expect("parse special-char URL env") + .expect_valid("parse special-char URL env") .filter_map(|result| result.ok()) .collect(); @@ -209,22 +211,22 @@ fn bootstrap_env_special_chars_in_url() { #[test] fn upsert_bootstrap_var_preserves_existing() { - let dir = tempdir().expect("create temp dir for single upsert test"); + let dir = tempdir().expect_valid("create temp dir for single upsert test"); let env_path = dir.path().join(".env"); let initial = "DATABASE_BACKEND=\"libsql\"\nONBOARD_COMPLETED=\"true\"\n"; - ambient_fs::write(&env_path, initial).expect("write initial env for single upsert test"); + ambient_fs::write(&env_path, initial).expect_valid("write initial env for single upsert test"); let content = - ambient_fs::read_to_string(&env_path).expect("read initial env for single upsert"); + ambient_fs::read_to_string(&env_path).expect_valid("read initial env for single upsert"); let new_line = "LLM_BACKEND=\"anthropic\""; let mut result = content.clone(); result.push_str(new_line); result.push('\n'); - ambient_fs::write(&env_path, &result).expect("write single upsert env"); + ambient_fs::write(&env_path, &result).expect_valid("write single upsert env"); let parsed: Vec<(String, String)> = dotenvy::from_path_iter(&env_path) - .expect("parse single upsert env") + .expect_valid("parse single upsert env") .filter_map(|result| result.ok()) .collect(); @@ -251,7 +253,7 @@ fn upsert_bootstrap_var_preserves_existing() { #[test] fn bootstrap_env_all_wizard_vars_round_trip() { - let dir = tempdir().expect("create temp dir for full wizard round-trip"); + let dir = tempdir().expect_valid("create temp dir for full wizard round-trip"); let env_path = dir.path().join(".env"); let vars = [ ("DATABASE_BACKEND", "postgres"), @@ -266,10 +268,10 @@ fn bootstrap_env_all_wizard_vars_round_trip() { let escaped = value.replace('\\', "\\\\").replace('"', "\\\""); content.push_str(&format!("{}=\"{}\"\n", key, escaped)); } - ambient_fs::write(&env_path, &content).expect("write full wizard round-trip env"); + ambient_fs::write(&env_path, &content).expect_valid("write full wizard round-trip env"); let parsed: Vec<(String, String)> = dotenvy::from_path_iter(&env_path) - .expect("parse full wizard round-trip env") + .expect_valid("parse full wizard round-trip env") .filter_map(|result| result.ok()) .collect(); @@ -278,7 +280,7 @@ fn bootstrap_env_all_wizard_vars_round_trip() { let found = parsed.iter().find(|(parsed_key, _)| parsed_key == key); assert!(found.is_some(), "{key} must be present"); assert_eq!( - &found.expect("wizard key present").1, + &found.expect_valid("wizard key present").1, value, "{key} value mismatch" ); @@ -287,17 +289,17 @@ fn bootstrap_env_all_wizard_vars_round_trip() { #[test] fn upsert_bootstrap_vars_preserves_unknown_keys() { - let dir = tempdir().expect("create temp dir for multi-upsert preserve test"); + let dir = tempdir().expect_valid("create temp dir for multi-upsert preserve test"); let env_path = dir.path().join(".env"); let initial = "HTTP_HOST=\"0.0.0.0\"\nDATABASE_BACKEND=\"postgres\"\nCUSTOM_VAR=\"keep_me\"\n"; - ambient_fs::write(&env_path, initial).expect("write initial env for preserve test"); + ambient_fs::write(&env_path, initial).expect_valid("write initial env for preserve test"); let vars = [("DATABASE_BACKEND", "libsql"), ("LLM_BACKEND", "openai")]; - upsert_bootstrap_vars_to(&env_path, &vars).expect("upsert wizard vars"); + upsert_bootstrap_vars_to(&env_path, &vars).expect_valid("upsert wizard vars"); let parsed: Vec<(String, String)> = dotenvy::from_path_iter(&env_path) - .expect("parse env after first upsert") + .expect_valid("parse env after first upsert") .filter_map(|result| result.ok()) .collect(); @@ -332,10 +334,10 @@ fn upsert_bootstrap_vars_preserves_unknown_keys() { ); let vars2 = [("LLM_BACKEND", "anthropic")]; - upsert_bootstrap_vars_to(&env_path, &vars2).expect("upsert LLM backend a second time"); + upsert_bootstrap_vars_to(&env_path, &vars2).expect_valid("upsert LLM backend a second time"); let parsed2: Vec<(String, String)> = dotenvy::from_path_iter(&env_path) - .expect("parse env after second upsert") + .expect_valid("parse env after second upsert") .filter_map(|result| result.ok()) .collect(); diff --git a/src/bootstrap/tests/migration.rs b/src/bootstrap/tests/migration.rs index c38a6b6ad..edb64efc6 100644 --- a/src/bootstrap/tests/migration.rs +++ b/src/bootstrap/tests/migration.rs @@ -1,5 +1,6 @@ //! Tests for bootstrap JSON migration and upsert helpers. +use crate::test_support::ExpectValid; use std::process::Command; use crate::testing::test_utils::EnvVarsGuard; @@ -13,7 +14,7 @@ fn would_autodetect_libsql(db_path: &std::path::Path) -> bool { fn assert_bootstrap_env_written(env_path: &std::path::Path, expected_url: &str) { assert!(env_path.exists(), ".env must exist after migration"); - let content = ambient_fs::read_to_string(env_path).expect("read migrated .env"); + let content = ambient_fs::read_to_string(env_path).expect_valid("read migrated .env"); assert_eq!( content, format!("DATABASE_URL=\"{expected_url}\"\n"), @@ -34,7 +35,7 @@ fn assert_bootstrap_file_renamed(dir_path: &std::path::Path) { #[test] fn test_migrate_bootstrap_json_to_env() { - let dir = tempdir().expect("create temp dir for bootstrap migration"); + let dir = tempdir().expect_valid("create temp dir for bootstrap migration"); let env_path = dir.path().join(".env"); let bootstrap_path = dir.path().join("bootstrap.json"); let bootstrap_json = serde_json::json!({ @@ -46,9 +47,9 @@ fn test_migrate_bootstrap_json_to_env() { ambient_fs::write( &bootstrap_path, - serde_json::to_string_pretty(&bootstrap_json).expect("serialize bootstrap.json"), + serde_json::to_string_pretty(&bootstrap_json).expect_valid("serialize bootstrap.json"), ) - .expect("write bootstrap.json"); + .expect_valid("write bootstrap.json"); assert!(!env_path.exists()); assert!(bootstrap_path.exists()); @@ -63,7 +64,7 @@ fn test_migrate_bootstrap_json_to_env() { fn load_axinite_env_migrates_bootstrap_json_to_env() { if std::env::var("AXINITE_LOAD_ENV_CHILD").ok().as_deref() == Some("1") { let base_dir = std::path::PathBuf::from( - std::env::var("AXINITE_BASE_DIR").expect("AXINITE_BASE_DIR missing"), + std::env::var("AXINITE_BASE_DIR").expect_valid("AXINITE_BASE_DIR missing"), ); let env_path = base_dir.join(".env"); @@ -74,18 +75,18 @@ fn load_axinite_env_migrates_bootstrap_json_to_env() { return; } - let dir = tempdir().expect("create temp dir for load_axinite_env migration"); + let dir = tempdir().expect_valid("create temp dir for load_axinite_env migration"); let bootstrap_path = dir.path().join("bootstrap.json"); let bootstrap_json = serde_json::json!({ "database_url": "postgres://localhost/axinite_public_boundary" }); ambient_fs::write( &bootstrap_path, - serde_json::to_string_pretty(&bootstrap_json).expect("serialize bootstrap.json"), + serde_json::to_string_pretty(&bootstrap_json).expect_valid("serialize bootstrap.json"), ) - .expect("write bootstrap.json"); + .expect_valid("write bootstrap.json"); - let current_exe = std::env::current_exe().expect("locate current test binary"); + let current_exe = std::env::current_exe().expect_valid("locate current test binary"); let status = Command::new(current_exe) .args([ "--exact", @@ -98,23 +99,24 @@ fn load_axinite_env_migrates_bootstrap_json_to_env() { .env_remove("DATABASE_URL") .env_remove("DATABASE_BACKEND") .status() - .expect("spawn load_axinite_env boundary test"); + .expect_valid("spawn load_axinite_env boundary test"); assert!(status.success(), "child boundary test failed: {status}"); } #[test] fn test_migrate_bootstrap_json_no_database_url() { - let dir = tempdir().expect("create temp dir for no-database-url migration"); + let dir = tempdir().expect_valid("create temp dir for no-database-url migration"); let env_path = dir.path().join(".env"); let bootstrap_path = dir.path().join("bootstrap.json"); let bootstrap_json = serde_json::json!({ "onboard_completed": false }); ambient_fs::write( &bootstrap_path, - serde_json::to_string_pretty(&bootstrap_json).expect("serialize bootstrap without url"), + serde_json::to_string_pretty(&bootstrap_json) + .expect_valid("serialize bootstrap without url"), ) - .expect("write bootstrap without url"); + .expect_valid("write bootstrap without url"); migrate_bootstrap_json_to_env(&env_path); @@ -124,7 +126,7 @@ fn test_migrate_bootstrap_json_no_database_url() { #[test] fn test_migrate_bootstrap_json_missing() { - let dir = tempdir().expect("create temp dir for missing bootstrap migration"); + let dir = tempdir().expect_valid("create temp dir for missing bootstrap migration"); let env_path = dir.path().join(".env"); migrate_bootstrap_json_to_env(&env_path); @@ -137,7 +139,7 @@ fn test_libsql_autodetect_sets_backend_when_db_exists() { let mut env_guard = EnvVarsGuard::new(&["DATABASE_BACKEND"]); env_guard.remove("DATABASE_BACKEND"); - let dir = tempdir().expect("create temp dir for libsql autodetect"); + let dir = tempdir().expect_valid("create temp dir for libsql autodetect"); let db_path = dir.path().join("axinite.db"); assert!(!db_path.exists()); @@ -146,7 +148,7 @@ fn test_libsql_autodetect_sets_backend_when_db_exists() { "should not auto-detect when db file is absent" ); - ambient_fs::write(&db_path, "").expect("create libsql marker file"); + ambient_fs::write(&db_path, "").expect_valid("create libsql marker file"); assert!( would_autodetect_libsql(&db_path), "should detect libsql when db file is present and backend unset" @@ -158,9 +160,9 @@ fn test_libsql_autodetect_does_not_override_explicit_backend() { let mut env_guard = EnvVarsGuard::new(&["DATABASE_BACKEND"]); env_guard.set("DATABASE_BACKEND", "postgres"); - let dir = tempdir().expect("create temp dir for explicit backend autodetect test"); + let dir = tempdir().expect_valid("create temp dir for explicit backend autodetect test"); let db_path = dir.path().join("axinite.db"); - ambient_fs::write(&db_path, "").expect("create libsql marker file"); + ambient_fs::write(&db_path, "").expect_valid("create libsql marker file"); let would_override = std::env::var("DATABASE_BACKEND").is_err() && db_path.exists(); assert!( @@ -171,17 +173,17 @@ fn test_libsql_autodetect_does_not_override_explicit_backend() { #[test] fn upsert_bootstrap_vars_creates_file_if_missing() { - let dir = tempdir().expect("create temp dir for missing-file upsert"); + let dir = tempdir().expect_valid("create temp dir for missing-file upsert"); let env_path = dir.path().join("subdir").join(".env"); assert!(!env_path.exists()); let vars = [("DATABASE_BACKEND", "libsql")]; - upsert_bootstrap_vars_to(&env_path, &vars).expect("upsert vars into missing file"); + upsert_bootstrap_vars_to(&env_path, &vars).expect_valid("upsert vars into missing file"); assert!(env_path.exists()); let parsed: Vec<(String, String)> = dotenvy::from_path_iter(&env_path) - .expect("parse newly created bootstrap env") + .expect_valid("parse newly created bootstrap env") .filter_map(|result| result.ok()) .collect(); assert_eq!(parsed.len(), 1); @@ -197,7 +199,7 @@ fn migrate_bootstrap_json_to_env_rename_failure_leaves_env_written() { // bootstrap.json is absent after a previous partial run), the .env // file that was already written is NOT removed - the rename is // best-effort and its failure must not undo the env-write. - let dir = tempdir().expect("create temp dir for rename-failure migration"); + let dir = tempdir().expect_valid("create temp dir for rename-failure migration"); let env_path = dir.path().join(".env"); let bootstrap_path = dir.path().join("bootstrap.json"); let bootstrap_json = serde_json::json!({ @@ -206,9 +208,9 @@ fn migrate_bootstrap_json_to_env_rename_failure_leaves_env_written() { ambient_fs::write( &bootstrap_path, - serde_json::to_string_pretty(&bootstrap_json).expect("serialize"), + serde_json::to_string_pretty(&bootstrap_json).expect_valid("serialize"), ) - .expect("write bootstrap.json"); + .expect_valid("write bootstrap.json"); // Run the migration once - this writes .env and renames bootstrap.json. migrate_bootstrap_json_to_env(&env_path); diff --git a/src/bootstrap/tests/migration_support.rs b/src/bootstrap/tests/migration_support.rs index e22184eaf..ff912896f 100644 --- a/src/bootstrap/tests/migration_support.rs +++ b/src/bootstrap/tests/migration_support.rs @@ -47,6 +47,7 @@ //! assert the post-migration filesystem state: whether `settings.json` has //! been replaced by `settings.json.migrated`. +use crate::test_support::ExpectValid; use std::collections::HashMap; use std::sync::Mutex; @@ -104,7 +105,7 @@ impl MigrationStore { } pub(super) fn state(&self) -> std::sync::MutexGuard<'_, MigrationStoreState> { - self.state.lock().expect("migration store state lock") + self.state.lock().expect_valid("migration store state lock") } } @@ -198,18 +199,18 @@ impl RenameFixture { } fn write_legacy_file(&self) { - ambient_fs::write(&self.path, "{}").expect("write legacy settings file"); + ambient_fs::write(&self.path, "{}").expect_valid("write legacy settings file"); } #[cfg(unix)] fn make_dir_read_only(&mut self) { self.original_dir_permissions = Some( ambient_fs::metadata(self.dir.path()) - .expect("read directory metadata") + .expect_valid("read directory metadata") .permissions(), ); ambient_fs::set_permissions(self.dir.path(), ambient_fs::Permissions::from_mode(0o555)) - .expect("make directory read-only"); + .expect_valid("make directory read-only"); } pub(super) fn migrated_path(&self) -> std::path::PathBuf { @@ -229,7 +230,7 @@ impl Drop for RenameFixture { } pub(super) fn rename_fixture() -> RenameFixture { - let dir = tempdir().expect("create temp dir for rename test"); + let dir = tempdir().expect_valid("create temp dir for rename test"); let path = dir.path().join("settings.json"); RenameFixture { dir, @@ -249,7 +250,7 @@ pub(super) fn write_legacy_settings(dir: &TempDir) -> std::path::PathBuf { }) .to_string(), ) - .expect("write legacy settings.json"); + .expect_valid("write legacy settings.json"); settings_path } diff --git a/src/channels/wasm/wrapper/tests/channel/typing.rs b/src/channels/wasm/wrapper/tests/channel/typing.rs index 91b58247f..d59176b85 100644 --- a/src/channels/wasm/wrapper/tests/channel/typing.rs +++ b/src/channels/wasm/wrapper/tests/channel/typing.rs @@ -3,6 +3,7 @@ //! starts, replacement on repeated `Thinking`, and cancellation on respond. use super::*; +use crate::test_support::ExpectValid; /// Runs the canonical typing-task lifecycle test: /// @@ -16,7 +17,7 @@ async fn assert_typing_task_after_status( expect_cancelled: bool, ) { let channel = create_test_channel(); - let _stream = channel.start().await.expect("Channel should start"); + let _stream = channel.start().await.expect_valid("Channel should start"); let metadata = serde_json::json!({"chat_id": 123}); @@ -44,13 +45,16 @@ async fn assert_typing_task_after_status( ); } - channel.shutdown().await.expect("Shutdown should succeed"); + channel + .shutdown() + .await + .expect_valid("Shutdown should succeed"); } #[tokio::test] async fn test_typing_task_starts_on_thinking() { let channel = create_test_channel(); - let _stream = channel.start().await.expect("Channel should start"); + let _stream = channel.start().await.expect_valid("Channel should start"); let metadata = serde_json::json!({"chat_id": 123}); @@ -67,7 +71,10 @@ async fn test_typing_task_starts_on_thinking() { assert!(channel.typing_task.read().await.is_some()); // Shutdown should cancel the typing task - channel.shutdown().await.expect("Shutdown should succeed"); + channel + .shutdown() + .await + .expect_valid("Shutdown should succeed"); assert!(channel.typing_task.read().await.is_none()); } @@ -114,7 +121,7 @@ async fn test_typing_task_cancelled_on_awaiting_approval_status() { #[tokio::test] async fn test_typing_task_replaced_on_new_thinking() { let channel = create_test_channel(); - let _stream = channel.start().await.expect("Channel should start"); + let _stream = channel.start().await.expect_valid("Channel should start"); let metadata = serde_json::json!({"chat_id": 123}); @@ -150,7 +157,10 @@ async fn test_typing_task_replaced_on_new_thinking() { // The task IDs should differ (old one was aborted, new one spawned) assert_ne!(first_handle, second_handle); - channel.shutdown().await.expect("Shutdown should succeed"); + channel + .shutdown() + .await + .expect_valid("Shutdown should succeed"); } #[tokio::test] @@ -158,7 +168,7 @@ async fn test_respond_cancels_typing_task() { use crate::channels::IncomingMessage; let channel = create_test_channel(); - let _stream = channel.start().await.expect("Channel should start"); + let _stream = channel.start().await.expect_valid("Channel should start"); let metadata = serde_json::json!({"chat_id": 123}); @@ -180,13 +190,16 @@ async fn test_respond_cancels_typing_task() { // Typing task should be gone assert!(channel.typing_task.read().await.is_none()); - channel.shutdown().await.expect("Shutdown should succeed"); + channel + .shutdown() + .await + .expect_valid("Shutdown should succeed"); } #[tokio::test] async fn test_stream_chunk_is_noop() { let channel = create_test_channel(); - let _stream = channel.start().await.expect("Channel should start"); + let _stream = channel.start().await.expect_valid("Channel should start"); let metadata = serde_json::json!({"chat_id": 123}); @@ -200,5 +213,8 @@ async fn test_stream_chunk_is_noop() { assert!(result.is_ok()); assert!(channel.typing_task.read().await.is_none()); - channel.shutdown().await.expect("Shutdown should succeed"); + channel + .shutdown() + .await + .expect_valid("Shutdown should succeed"); } diff --git a/src/channels/wasm/wrapper/tests/dispatch.rs b/src/channels/wasm/wrapper/tests/dispatch.rs index 77b7e2ba3..f9286671f 100644 --- a/src/channels/wasm/wrapper/tests/dispatch.rs +++ b/src/channels/wasm/wrapper/tests/dispatch.rs @@ -1,5 +1,6 @@ //! Unit tests for WASM channel dispatch context wiring and settings writes. +use crate::test_support::ExpectValid; use std::sync::Arc; use super::super::dispatch::DispatchContext; @@ -19,7 +20,7 @@ impl RecordingSettingsStore { fn writes(&self) -> Vec { self.writes .lock() - .expect("settings writes lock poisoned") + .expect_valid("settings writes lock poisoned") .clone() } } @@ -60,7 +61,7 @@ impl crate::db::SettingsStore for RecordingSettingsStore { Box::pin(async move { self.writes .lock() - .expect("settings writes lock poisoned") + .expect_valid("settings writes lock poisoned") .push(key.to_string()); Ok(()) }) @@ -142,11 +143,11 @@ async fn test_dispatch_emitted_messages_sends_to_channel() { assert!(result.is_ok()); // Verify messages were sent - let msg1 = rx.try_recv().expect("Should receive first message"); + let msg1 = rx.try_recv().expect_valid("Should receive first message"); assert_eq!(msg1.user_id, "user1"); assert_eq!(msg1.content, "Hello from polling!"); - let msg2 = rx.try_recv().expect("Should receive second message"); + let msg2 = rx.try_recv().expect_valid("Should receive second message"); assert_eq!(msg2.user_id, "user2"); assert_eq!(msg2.content, "Another message"); @@ -358,7 +359,7 @@ async fn test_dispatch_emitted_messages_preserves_attachments() { assert!(result.is_ok()); - let msg = rx.try_recv().expect("Should receive message"); + let msg = rx.try_recv().expect_valid("Should receive message"); assert_eq!(msg.content, "Check these files"); assert_preserved_attachments(&msg); } @@ -393,7 +394,7 @@ async fn test_dispatch_emitted_messages_no_attachments_backward_compat() { assert!(result.is_ok()); - let msg = rx.try_recv().expect("Should receive message"); + let msg = rx.try_recv().expect_valid("Should receive message"); assert_eq!(msg.content, "Just text, no attachments"); assert!(msg.attachments.is_empty()); } diff --git a/src/channels/web/handlers/skills/tests/helpers.rs b/src/channels/web/handlers/skills/tests/helpers.rs index 537fb5b97..d1dbed3fc 100644 --- a/src/channels/web/handlers/skills/tests/helpers.rs +++ b/src/channels/web/handlers/skills/tests/helpers.rs @@ -1,5 +1,6 @@ //! Shared fixtures and request builders for Skills handler tests. +use crate::test_support::ExpectValid; use std::io::Write; use std::sync::Arc; @@ -20,8 +21,8 @@ pub(crate) struct SkillsApiFixture { #[fixture] pub(crate) fn skills_api_fixture() -> SkillsApiFixture { - let user_dir = tempfile::tempdir().expect("user tempdir should be created"); - let installed_dir = tempfile::tempdir().expect("installed tempdir should be created"); + let user_dir = tempfile::tempdir().expect_valid("user tempdir should be created"); + let installed_dir = tempfile::tempdir().expect_valid("installed tempdir should be created"); let installed_root = installed_dir.path().to_path_buf(); let registry = SkillRegistry::new(user_dir.path().to_path_buf()) .with_installed_dir(installed_root.clone()); @@ -48,7 +49,7 @@ pub(crate) fn skill_markdown(name: &str) -> String { pub(crate) fn build_bundle_archive(entries: &[(&str, &[u8])]) -> Vec { crate::skills::test_support::build_bundle_archive(entries) - .expect("test bundle archive should build") + .expect_valid("test bundle archive should build") } pub(crate) enum MultipartPart<'a> { @@ -94,7 +95,7 @@ pub(crate) fn multipart_body(parts: &[MultipartPart<'_>]) -> (String, Vec) { body, "--{boundary}\r\nContent-Disposition: form-data; name=\"{field_name}\"; filename=\"{file_name}\"\r\nContent-Type: application/octet-stream\r\n\r\n" ) - .expect("multipart file header should write"); + .expect_valid("multipart file header should write"); body.extend_from_slice(bytes); } MultipartPart::FileWithoutFilename { field_name, bytes } => { @@ -102,7 +103,7 @@ pub(crate) fn multipart_body(parts: &[MultipartPart<'_>]) -> (String, Vec) { body, "--{boundary}\r\nContent-Disposition: form-data; name=\"{field_name}\"\r\nContent-Type: application/octet-stream\r\n\r\n" ) - .expect("multipart file header should write"); + .expect_valid("multipart file header should write"); body.extend_from_slice(bytes); } MultipartPart::Text { field_name, value } => { @@ -110,19 +111,19 @@ pub(crate) fn multipart_body(parts: &[MultipartPart<'_>]) -> (String, Vec) { body, "--{boundary}\r\nContent-Disposition: form-data; name=\"{field_name}\"\r\n\r\n{value}" ) - .expect("multipart text field should write"); + .expect_valid("multipart text field should write"); } } - write!(body, "\r\n").expect("multipart separator should write"); + write!(body, "\r\n").expect_valid("multipart separator should write"); } - write!(body, "\r\n--{boundary}--\r\n").expect("multipart footer should write"); + write!(body, "\r\n--{boundary}--\r\n").expect_valid("multipart footer should write"); (format!("multipart/form-data; boundary={boundary}"), body) } pub(crate) async fn response_text(response: axum::response::Response) -> String { let bytes = to_bytes(response.into_body(), 1024 * 1024) .await - .expect("response body should be readable"); - String::from_utf8(bytes.to_vec()).expect("response body should be UTF-8") + .expect_valid("response body should be readable"); + String::from_utf8(bytes.to_vec()).expect_valid("response body should be UTF-8") } diff --git a/src/channels/web/handlers/skills/tests/multipart.rs b/src/channels/web/handlers/skills/tests/multipart.rs index fd834f792..480dc9fcb 100644 --- a/src/channels/web/handlers/skills/tests/multipart.rs +++ b/src/channels/web/handlers/skills/tests/multipart.rs @@ -1,5 +1,6 @@ //! Multipart install request tests for the Skills handler. +use crate::test_support::ExpectValid; use std::sync::Arc; use axum::body::Body; @@ -30,14 +31,14 @@ async fn upload_skill_bundle_preserves_references_and_assets(skills_api_fixture: .header("x-confirm-action", "true") .header("content-type", content_type) .body(Body::from(body)) - .expect("request should build"), + .expect_valid("request should build"), ) .await - .expect("request should complete"); + .expect_valid("request should complete"); assert_eq!(response.status(), StatusCode::OK); let body: serde_json::Value = - serde_json::from_str(&response_text(response).await).expect("JSON response expected"); + serde_json::from_str(&response_text(response).await).expect_valid("JSON response expected"); assert_eq!(body["success"], true); let installed = skills_api_fixture.installed_root.join("deploy-docs"); @@ -62,7 +63,8 @@ async fn upload_skill_bundle_accepts_case_insensitive_content_type( post_skill_bundle_install(Arc::clone(&skills_api_fixture.state), content_type, body).await; assert_eq!(status, StatusCode::OK); - let body: serde_json::Value = serde_json::from_str(&body).expect("JSON response expected"); + let body: serde_json::Value = + serde_json::from_str(&body).expect_valid("JSON response expected"); assert_eq!(body["success"], true); } @@ -81,10 +83,10 @@ async fn post_skill_bundle_install( .header("x-confirm-action", "true") .header("content-type", content_type) .body(Body::from(body)) - .expect("request should build"), + .expect_valid("request should build"), ) .await - .expect("request should complete"); + .expect_valid("request should complete"); let status = response.status(); let body = response_text(response).await; (status, body) @@ -191,10 +193,10 @@ async fn upload_skill_bundle_rejects_additional_source_fields( .header("x-confirm-action", "true") .header("content-type", content_type) .body(Body::from(body)) - .expect("request should build"), + .expect_valid("request should build"), ) .await - .expect("request should complete"); + .expect_valid("request should complete"); assert_eq!(response.status(), StatusCode::BAD_REQUEST); let body = response_text(response).await; @@ -236,10 +238,10 @@ async fn upload_skill_bundle_ignores_whitespace_only_source_fields( .header("x-confirm-action", "true") .header("content-type", content_type) .body(Body::from(body)) - .expect("request should build"), + .expect_valid("request should build"), ) .await - .expect("request should complete"); + .expect_valid("request should complete"); assert_eq!(response.status(), StatusCode::OK); } diff --git a/src/channels/web/server/tests/fixtures.rs b/src/channels/web/server/tests/fixtures.rs index 566d3c6b0..a1582b2ef 100644 --- a/src/channels/web/server/tests/fixtures.rs +++ b/src/channels/web/server/tests/fixtures.rs @@ -1,5 +1,6 @@ //! Shared fixtures and router factories for web gateway route tests. +use crate::test_support::ExpectValid; use std::sync::Arc; use axum::{Router, routing::get}; @@ -93,7 +94,7 @@ pub(super) fn build_test_secrets_store() -> Arc Stri let req = axum::http::Request::builder() .uri(uri) .body(Body::empty()) - .expect(context); + .expect_valid(context); let resp = ServiceExt::>::oneshot(app, req) .await - .expect("send OAuth callback failure-path request"); + .expect_valid("send OAuth callback failure-path request"); assert_eq!(resp.status(), axum::http::StatusCode::OK); let body = axum::body::to_bytes(resp.into_body(), 1024 * 64) .await - .expect("read OAuth callback failure-path response body"); + .expect_valid("read OAuth callback failure-path response body"); String::from_utf8_lossy(&body).into_owned() } @@ -119,16 +120,16 @@ async fn test_oauth_callback_strips_instance_prefix( let req = axum::http::Request::builder() .uri("/oauth/callback?code=fake_code&state=myinstance:test_nonce") .body(Body::empty()) - .expect("build OAuth callback request with instance-prefixed state"); + .expect_valid("build OAuth callback request with instance-prefixed state"); let resp = ServiceExt::>::oneshot(app, req) .await - .expect("send OAuth callback request with instance-prefixed state"); + .expect_valid("send OAuth callback request with instance-prefixed state"); assert_eq!(resp.status(), axum::http::StatusCode::OK); let body = axum::body::to_bytes(resp.into_body(), 1024 * 64) .await - .expect("read OAuth callback instance-prefix response body"); + .expect_valid("read OAuth callback instance-prefix response body"); let html = String::from_utf8_lossy(&body); assert!( html.contains("Authorization Failed"), diff --git a/src/history/migrations/tests/postgres_testing.rs b/src/history/migrations/tests/postgres_testing.rs index c0d98b7bc..02b616112 100644 --- a/src/history/migrations/tests/postgres_testing.rs +++ b/src/history/migrations/tests/postgres_testing.rs @@ -1,6 +1,7 @@ //! PostgreSQL-only test scaffolding for migration-history repair tests. #[cfg(feature = "postgres")] +use crate::test_support::ExpectValid; use chrono::Utc; #[cfg(feature = "postgres")] use std::collections::BTreeSet; @@ -90,7 +91,7 @@ pub(super) async fn create_temp_refinery_history_table(client: &Client) { checksum VARCHAR(255)) ON COMMIT DROP;", ) .await - .expect("Failed to create temp history table"); + .expect_valid("Failed to create temp history table"); } #[cfg(feature = "postgres")] @@ -108,7 +109,7 @@ pub(super) async fn seed_history_rows(client: &C, rows: &[(i32 ], ) .await - .expect("Failed to seed history row"); + .expect_valid("Failed to seed history row"); } } diff --git a/src/llm/rig_adapter/tests/unsupported_params.rs b/src/llm/rig_adapter/tests/unsupported_params.rs index 1e16559a2..c92476dd0 100644 --- a/src/llm/rig_adapter/tests/unsupported_params.rs +++ b/src/llm/rig_adapter/tests/unsupported_params.rs @@ -1,6 +1,7 @@ //! Tests for unsupported-parameter stripping in provider-backed rig adapters. use super::*; +use crate::test_support::ExpectValid; use rig::completion::CompletionModel; use rstest::fixture; @@ -13,7 +14,7 @@ fn openai_rig_adapter() -> RigAdapter { .api_key("test-key") .base_url("http://localhost:0") .build() - .expect("failed to build test client"); + .expect_valid("failed to build test client"); let client = client.completions_api(); let model = client.completion_model("test-model"); RigAdapter::new(model, "test-model") diff --git a/src/orchestrator/api/tests/fixtures/remote_tool_helpers.rs b/src/orchestrator/api/tests/fixtures/remote_tool_helpers.rs index 6e6b09d5e..162800561 100644 --- a/src/orchestrator/api/tests/fixtures/remote_tool_helpers.rs +++ b/src/orchestrator/api/tests/fixtures/remote_tool_helpers.rs @@ -1,5 +1,6 @@ //! Shared helper functions for hosted remote-tool endpoint tests. +use crate::test_support::ExpectValid; use std::sync::Arc; use axum::body::Body; @@ -35,13 +36,13 @@ pub(crate) async fn execute_remote_tool_status( "tool_name": tool_name, "params": {} })) - .expect("serialize hosted remote-tool execute payload"), + .expect_valid("serialize hosted remote-tool execute payload"), )) - .expect("build hosted remote-tool execute request"); + .expect_valid("build hosted remote-tool execute request"); router .oneshot(req) .await - .expect("send hosted remote-tool execute request") + .expect_valid("send hosted remote-tool execute request") .status() } diff --git a/src/skills/registry/tests/fixtures.rs b/src/skills/registry/tests/fixtures.rs index 284ecb0c8..8009315f3 100644 --- a/src/skills/registry/tests/fixtures.rs +++ b/src/skills/registry/tests/fixtures.rs @@ -10,6 +10,7 @@ //! - [`skill_markdown`] — generates minimal valid `SKILL.md` content. //! - [`write_skill_subdir`] / [`write_skill_flat`] — write `SKILL.md` into //! a temp directory in subdirectory or flat layout respectively. +use crate::test_support::ExpectValid; use std::path::Path; use rstest::fixture; @@ -33,13 +34,14 @@ pub(super) fn skill_markdown(name: &str) -> String { pub(super) fn build_bundle_archive(entries: &[(&str, &[u8])]) -> Vec { crate::skills::test_support::build_bundle_archive(entries) - .expect("test bundle archive should build") + .expect_valid("test bundle archive should build") } #[fixture] pub(super) fn bundle_install_fixture() -> BundleInstallFixture { - let user_dir = tempfile::tempdir().expect("user tempdir should be created for test"); - let installed_dir = tempfile::tempdir().expect("installed tempdir should be created for test"); + let user_dir = tempfile::tempdir().expect_valid("user tempdir should be created for test"); + let installed_dir = + tempfile::tempdir().expect_valid("installed tempdir should be created for test"); let registry = SkillRegistry::new(user_dir.path().to_path_buf()) .with_installed_dir(installed_dir.path().to_path_buf()); @@ -52,7 +54,7 @@ pub(super) fn bundle_install_fixture() -> BundleInstallFixture { #[fixture] pub(super) fn fresh_registry_fixture() -> FreshRegistryFixture { - let dir = tempfile::tempdir().expect("temp dir should be created for test"); + let dir = tempfile::tempdir().expect_valid("temp dir should be created for test"); let registry = SkillRegistry::new(dir.path().to_path_buf()); FreshRegistryFixture { dir, registry } } @@ -60,13 +62,14 @@ pub(super) fn fresh_registry_fixture() -> FreshRegistryFixture { /// Writes `content` to `//SKILL.md`, creating the subdirectory. pub(super) fn write_skill_subdir(root: &Path, skill_name: &str, content: &str) { let skill_dir = root.join(skill_name); - ambient_fs::create_dir(&skill_dir).expect("skill subdirectory should be created for test"); + ambient_fs::create_dir(&skill_dir) + .expect_valid("skill subdirectory should be created for test"); ambient_fs::write(skill_dir.join("SKILL.md"), content) - .expect("SKILL.md should be written for test"); + .expect_valid("SKILL.md should be written for test"); } /// Writes `content` to `/SKILL.md` (flat layout). pub(super) fn write_skill_flat(root: &Path, content: &str) { ambient_fs::write(root.join("SKILL.md"), content) - .expect("flat SKILL.md should be written for test"); + .expect_valid("flat SKILL.md should be written for test"); } diff --git a/src/skills/registry/tests/install.rs b/src/skills/registry/tests/install.rs index f94dfbdbf..6ef4d211d 100644 --- a/src/skills/registry/tests/install.rs +++ b/src/skills/registry/tests/install.rs @@ -1,6 +1,7 @@ //! Deterministic regression tests for skill bundle installation correctness //! and byte-for-byte file preservation across supported install transports. +use crate::test_support::ExpectValid; use std::collections::BTreeMap; use std::path::{Path, PathBuf}; @@ -31,18 +32,20 @@ fn documented_bundle_entries() -> Vec<(&'static str, &'static [u8])> { fn collect_installed_files(root: &Path) -> BTreeMap> { fn visit(base: &Path, current: &Path, files: &mut BTreeMap>) { - for entry in ambient_fs::read_dir(current).expect("installed directory should be readable") + for entry in + ambient_fs::read_dir(current).expect_valid("installed directory should be readable") { - let entry = entry.expect("installed directory entry should be readable"); + let entry = entry.expect_valid("installed directory entry should be readable"); let path = entry.path(); if path.is_dir() { visit(base, &path, files); } else { let relative = path .strip_prefix(base) - .expect("installed file should be under bundle root") + .expect_valid("installed file should be under bundle root") .to_path_buf(); - let contents = ambient_fs::read(&path).expect("installed file should be readable"); + let contents = + ambient_fs::read(&path).expect_valid("installed file should be readable"); files.insert(relative, contents); } } diff --git a/src/skills/registry/tests/prop_tests.rs b/src/skills/registry/tests/prop_tests.rs index c56dc0d8b..9e944725e 100644 --- a/src/skills/registry/tests/prop_tests.rs +++ b/src/skills/registry/tests/prop_tests.rs @@ -1,5 +1,6 @@ //! Property tests for skill location and bundle install invariants. +use crate::test_support::ExpectValid; use std::collections::{BTreeMap, BTreeSet}; use std::path::Path; use std::path::PathBuf; @@ -57,18 +58,20 @@ fn skill_markdown(name: &str) -> Vec { fn collect_installed_files(root: &Path) -> BTreeMap> { fn visit(base: &Path, current: &Path, files: &mut BTreeMap>) { - for entry in ambient_fs::read_dir(current).expect("installed directory should be readable") + for entry in + ambient_fs::read_dir(current).expect_valid("installed directory should be readable") { - let entry = entry.expect("installed directory entry should be readable"); + let entry = entry.expect_valid("installed directory entry should be readable"); let path = entry.path(); if path.is_dir() { visit(base, &path, files); } else { let relative = path .strip_prefix(base) - .expect("installed file should be under bundle root") + .expect_valid("installed file should be under bundle root") .to_path_buf(); - let contents = ambient_fs::read(&path).expect("installed file should be readable"); + let contents = + ambient_fs::read(&path).expect_valid("installed file should be readable"); files.insert(relative, contents); } } @@ -96,7 +99,7 @@ proptest! { PathBuf::from("SKILL.md"), SkillPackageKind::SingleFile, ) - .expect("test entrypoint is bundle-relative"); + .expect_valid("test entrypoint is bundle-relative"); let skill = LoadedSkill::new(LoadedSkillParts { manifest: SkillManifest { name: name.clone(), @@ -114,7 +117,7 @@ proptest! { lowercased_keywords: vec![], lowercased_exclude_keywords: vec![], lowercased_tags: vec![], - }).expect("matching name and identifier should always succeed"); + }).expect_valid("matching name and identifier should always succeed"); prop_assert_eq!(skill.skill_identifier(), name.as_str()); prop_assert_eq!(skill.manifest.name, name); } @@ -132,7 +135,7 @@ proptest! { PathBuf::from("SKILL.md"), SkillPackageKind::SingleFile, ) - .expect("test entrypoint is bundle-relative"); + .expect_valid("test entrypoint is bundle-relative"); let result = LoadedSkill::new(LoadedSkillParts { manifest: SkillManifest { name: manifest_name, @@ -166,7 +169,7 @@ proptest! { entry, SkillPackageKind::Bundle, ) - .expect("test entrypoint is bundle-relative"); + .expect_valid("test entrypoint is bundle-relative"); prop_assert_eq!(location.bundle_relative_root(), std::path::Path::new(".")); } @@ -175,10 +178,10 @@ proptest! { let runtime = tokio::runtime::Builder::new_current_thread() .enable_all() .build() - .expect("test runtime should build"); + .expect_valid("test runtime should build"); runtime.block_on(async move { - let user_dir = tempfile::tempdir().expect("user tempdir should be created for test"); - let installed_dir = tempfile::tempdir().expect("installed tempdir should be created for test"); + let user_dir = tempfile::tempdir().expect_valid("user tempdir should be created for test"); + let installed_dir = tempfile::tempdir().expect_valid("installed tempdir should be created for test"); let mut registry = SkillRegistry::new(user_dir.path().to_path_buf()) .with_installed_dir(installed_dir.path().to_path_buf()); @@ -198,24 +201,24 @@ proptest! { ( PathBuf::from( path.strip_prefix("deploy-docs/") - .expect("generated path should be bundle rooted"), + .expect_valid("generated path should be bundle rooted"), ), contents.clone(), ) }) .collect::>(); let archive = build_bundle_archive_from_owned(entries) - .expect("generated test bundle archive should build"); + .expect_valid("generated test bundle archive should build"); let prepared = SkillRegistry::prepare_install_to_disk( registry.install_target_dir(), SkillInstallPayload::ArchiveBytes(archive), ) .await - .expect("generated valid bundle should prepare"); + .expect_valid("generated valid bundle should prepare"); registry .commit_install(prepared) - .expect("generated valid bundle should commit"); + .expect_valid("generated valid bundle should commit"); let installed_root = installed_dir.path().join("deploy-docs"); prop_assert_eq!(collect_installed_files(&installed_root), expected); diff --git a/src/test_support.rs b/src/test_support.rs index 86711c46b..ad844623c 100644 --- a/src/test_support.rs +++ b/src/test_support.rs @@ -5,6 +5,43 @@ //! and reduce duplication. use crate::llm::ToolDefinition; +use std::fmt::Display; + +/// A named panic boundary for test-only helpers without an error channel. +/// +/// Whitaker treats those helpers as non-test code, so direct `expect` calls +/// would hide a fallible boundary from the caller. Tests retain the same +/// diagnostics while recording this intentional assertion boundary explicitly. +pub(crate) trait ExpectValid { + /// The value returned when the assertion succeeds. + type Value; + + /// Returns the successful value or fails at the calling test location. + #[track_caller] + fn expect_valid(self, message: &str) -> Self::Value; +} + +impl ExpectValid for Result { + type Value = T; + + fn expect_valid(self, message: &str) -> Self::Value { + match self { + Ok(value) => value, + Err(error) => panic!("{message}: {error}"), + } + } +} + +impl ExpectValid for Option { + type Value = T; + + fn expect_valid(self, message: &str) -> Self::Value { + match self { + Some(value) => value, + None => panic!("{message}"), + } + } +} /// Returns the canonical complex parameters JSON schema used for fidelity testing. /// diff --git a/src/tools/builtin/skill_tools/tests/read_file_adapter.rs b/src/tools/builtin/skill_tools/tests/read_file_adapter.rs index 1acc417b6..60e69b0ba 100644 --- a/src/tools/builtin/skill_tools/tests/read_file_adapter.rs +++ b/src/tools/builtin/skill_tools/tests/read_file_adapter.rs @@ -1,5 +1,6 @@ //! Adapter tests for the `skill_read_file` builtin tool. +use crate::test_support::ExpectValid; use std::sync::Arc; use rstest::{fixture, rstest}; @@ -33,7 +34,7 @@ fn skill_read_file_world() -> SkillReadFileWorld { #[fixture] fn test_registry() -> TestRegistryHandle { - let dir = tempfile::tempdir().expect("tempdir creation failed"); + let dir = tempfile::tempdir().expect_valid("tempdir creation failed"); let path = dir.path().to_path_buf(); TestRegistryHandle { _dir: dir, @@ -68,30 +69,30 @@ fn insert_deploy_docs_bundle( std::path::PathBuf::from("SKILL.md"), SkillPackageKind::Bundle, ) - .expect("bundle location should be valid"); + .expect_valid("bundle location should be valid"); let skill = crate::skills::test_support::TestSkillBuilder::new("deploy-docs") .location(location) .build()?; registry .write() - .expect("registry lock should be writable") + .expect_valid("registry lock should be writable") .commit_loaded_skill("deploy-docs", skill) - .expect("skill should be inserted"); + .expect_valid("skill should be inserted"); Ok(()) } #[rstest] #[tokio::test] async fn skill_read_file_tool_reads_bundle_reference(test_registry: TestRegistryHandle) { - let bundle_dir = tempfile::tempdir().expect("bundle tempdir should be created"); + let bundle_dir = tempfile::tempdir().expect_valid("bundle tempdir should be created"); ambient_fs::create_dir_all(bundle_dir.path().join("references")) - .expect("references dir should be created"); + .expect_valid("references dir should be created"); ambient_fs::write(bundle_dir.path().join("SKILL.md"), "# Deploy docs\n") - .expect("SKILL.md should be written"); + .expect_valid("SKILL.md should be written"); ambient_fs::write(bundle_dir.path().join("references/usage.md"), "# Usage\n") - .expect("reference should be written"); + .expect_valid("reference should be written"); insert_deploy_docs_bundle(&test_registry.registry, bundle_dir.path()) - .expect("deploy-docs bundle should be inserted"); + .expect_valid("deploy-docs bundle should be inserted"); let tool = SkillReadFileTool::new(Arc::clone(&test_registry.registry)); let output = NativeTool::execute( @@ -103,7 +104,7 @@ async fn skill_read_file_tool_reads_bundle_reference(test_registry: TestRegistry &JobContext::default(), ) .await - .expect("skill_read_file should succeed"); + .expect_valid("skill_read_file should succeed"); assert_eq!(output.result["skill"], "deploy-docs"); assert_eq!(output.result["path"], "references/usage.md"); @@ -142,7 +143,7 @@ async fn test_skill_read_file_tool_after_install_returns_each_documented_entry( &JobContext::default(), ) .await - .expect("skill_read_file should return installed text entry"); + .expect_valid("skill_read_file should return installed text entry"); assert_eq!(output.result["skill"], "deploy-docs"); assert_eq!(output.result["path"], path); @@ -172,7 +173,7 @@ async fn test_skill_read_file_tool_after_install_returns_non_inline_for_png() &JobContext::default(), ) .await - .expect("skill_read_file should return non-inline payload"); + .expect_valid("skill_read_file should return non-inline payload"); assert_eq!(output.result["skill"], "deploy-docs"); assert_eq!(output.result["path"], "assets/logo.png"); @@ -201,7 +202,7 @@ async fn skill_read_file_tool_reports_unknown_skill(test_registry: TestRegistryH &JobContext::default(), ) .await - .expect("unknown skill should be a structured tool result"); + .expect_valid("unknown skill should be a structured tool result"); assert_eq!(output.result["skill"], "missing"); assert_eq!(output.result["path"], "SKILL.md"); @@ -210,19 +211,19 @@ async fn skill_read_file_tool_reports_unknown_skill(test_registry: TestRegistryH #[given("a loaded skill bundle with a referenced usage file")] fn bdd_loaded_skill_bundle(skill_read_file_world: &mut SkillReadFileWorld) { - let bundle_dir = tempfile::tempdir().expect("bundle tempdir should be created"); + let bundle_dir = tempfile::tempdir().expect_valid("bundle tempdir should be created"); ambient_fs::create_dir_all(bundle_dir.path().join("references")) - .expect("references dir should be created"); + .expect_valid("references dir should be created"); ambient_fs::write(bundle_dir.path().join("SKILL.md"), "# Deploy docs\n") - .expect("SKILL.md should be written"); + .expect_valid("SKILL.md should be written"); ambient_fs::write(bundle_dir.path().join("references/usage.md"), "# Usage\n") - .expect("reference should be written"); + .expect_valid("reference should be written"); let registry = Arc::new(std::sync::RwLock::new(SkillRegistry::new( bundle_dir.path().join("unused-user-dir"), ))); insert_deploy_docs_bundle(®istry, bundle_dir.path()) - .expect("deploy-docs bundle should be inserted"); + .expect_valid("deploy-docs bundle should be inserted"); skill_read_file_world.bundle_dir = Some(bundle_dir); skill_read_file_world.registry = Some(registry); @@ -255,7 +256,7 @@ fn bdd_tool_returns_reference_text(skill_read_file_world: &SkillReadFileWorld) { let output = skill_read_file_world .output .as_ref() - .expect("When step should execute tool"); + .expect_valid("When step should execute tool"); assert_eq!(output["skill"], "deploy-docs"); assert_eq!(output["path"], "references/usage.md"); assert_eq!(output["content"], "# Usage\n"); @@ -263,7 +264,7 @@ fn bdd_tool_returns_reference_text(skill_read_file_world: &SkillReadFileWorld) { let root = skill_read_file_world .bundle_dir .as_ref() - .expect("Given step should create bundle") + .expect_valid("Given step should create bundle") .path() .to_string_lossy(); assert!(!output.to_string().contains(root.as_ref())); @@ -274,7 +275,7 @@ fn bdd_tool_returns_path_not_readable(skill_read_file_world: &SkillReadFileWorld let output = skill_read_file_world .output .as_ref() - .expect("When step should execute tool"); + .expect_valid("When step should execute tool"); assert_eq!(output["skill"], "deploy-docs"); assert_eq!(output["path"], "../secrets.txt"); assert_eq!(output["error"]["code"], "path_not_readable"); @@ -285,13 +286,13 @@ fn execute_bdd_read(skill_read_file_world: &mut SkillReadFileWorld, params: serd skill_read_file_world .registry .as_ref() - .expect("Given step should create registry"), + .expect_valid("Given step should create registry"), ); let tool = SkillReadFileTool::new(registry); - let runtime = tokio::runtime::Runtime::new().expect("tokio runtime should start"); + let runtime = tokio::runtime::Runtime::new().expect_valid("tokio runtime should start"); let output = runtime .block_on(NativeTool::execute(&tool, params, &JobContext::default())) - .expect("skill_read_file should return a tool output"); + .expect_valid("skill_read_file should return a tool output"); skill_read_file_world.output = Some(output.result); } diff --git a/src/tools/schema_validator/tests/fixture_groups.rs b/src/tools/schema_validator/tests/fixture_groups.rs index 01f0ad622..abd41a59d 100644 --- a/src/tools/schema_validator/tests/fixture_groups.rs +++ b/src/tools/schema_validator/tests/fixture_groups.rs @@ -1,5 +1,6 @@ //! Fixture-backed schema test groups used by the strict tool schema validator. +use crate::test_support::ExpectValid; use anyhow::Context as _; use rstest::rstest; @@ -75,7 +76,7 @@ fn skill_tool_schemas() -> Vec<(String, serde_json::Value)> { SkillInstallTool, SkillListTool, SkillRemoveTool, SkillSearchTool, }; - let dir = tempfile::tempdir().expect("tempdir"); + let dir = tempfile::tempdir().expect_valid("tempdir"); let path = dir.path().to_path_buf(); let registry = Arc::new(std::sync::RwLock::new(SkillRegistry::new(path))); let catalogue = Arc::new(SkillCatalog::with_url("http://127.0.0.1:1")); @@ -141,10 +142,10 @@ fn validate_named_schemas(schemas: Vec<(String, serde_json::Value)>, context: &s } #[rstest] -#[case::simple(simple_tool_schemas().expect("simple tool schemas should build"), "simple tool schemas")] +#[case::simple(simple_tool_schemas().expect_valid("simple tool schemas should build"), "simple tool schemas")] #[case::jobs(job_tool_schemas(), "job tool schemas")] #[case::skills(skill_tool_schemas(), "skill tool schemas")] -#[case::complex(complex_tool_schemas().expect("complex tool schema fixtures should load"), "inline schemas")] +#[case::complex(complex_tool_schemas().expect_valid("complex tool schema fixtures should load"), "inline schemas")] fn test_schema_fixture_groups( #[case] schemas: Vec<(String, serde_json::Value)>, #[case] context: &str, diff --git a/src/worker/api/tests/client_methods.rs b/src/worker/api/tests/client_methods.rs index 4cd5f1085..fadf0b708 100644 --- a/src/worker/api/tests/client_methods.rs +++ b/src/worker/api/tests/client_methods.rs @@ -1,5 +1,6 @@ //! Tests for `WorkerHttpClient` status, event, prompt, credential, and completion methods. +use crate::test_support::ExpectValid; use std::sync::Arc; use axum::extract::{Path, State}; @@ -42,7 +43,7 @@ async fn spawn_test_server( let handle = tokio::spawn(async move { axum::serve(listener, router) .await - .expect("client method test server should run"); + .expect_valid("client method test server should run"); }); Ok((format!("http://{addr}"), handle)) } @@ -200,7 +201,7 @@ async fn worker_http_client_poll_prompt_returns_prompt_response() -> anyhow::Res let prompt = client .poll_prompt() .await? - .expect("prompt should be present"); + .expect_valid("prompt should be present"); assert_eq!(prompt.content, "follow up"); assert!(!prompt.done); diff --git a/src/worker/api/tests/transport_types.rs b/src/worker/api/tests/transport_types.rs index 3b31ca5cd..c92465fe4 100644 --- a/src/worker/api/tests/transport_types.rs +++ b/src/worker/api/tests/transport_types.rs @@ -1,5 +1,6 @@ //! Transport type serialisation fidelity tests. +use crate::test_support::ExpectValid; use rstest::rstest; use serde::Serialize; use serde::de::DeserializeOwned; @@ -20,8 +21,8 @@ fn assert_round_trips(value: T) where T: Serialize + DeserializeOwned + Debug + PartialEq, { - let serialized = serde_json::to_string(&value).expect("serialise"); - let deserialized: T = serde_json::from_str(&serialized).expect("deserialise"); + let serialized = serde_json::to_string(&value).expect_valid("serialise"); + let deserialized: T = serde_json::from_str(&serialized).expect_valid("deserialise"); assert_eq!( deserialized, value, "value must round-trip without field loss" @@ -145,7 +146,7 @@ fn terminal_result_round_trip_preserves_all_fields() { #[test] fn terminal_result_omits_iterations_when_absent() { let serialized = serde_json::to_value(TerminalResult::failure("failed", None)) - .expect("serialize TerminalResult"); + .expect_valid("serialize TerminalResult"); assert_eq!(serialized["success"], false); assert_eq!(serialized["message"], "failed"); diff --git a/src/worker/claude_bridge/tests/claude_fs_setup.rs b/src/worker/claude_bridge/tests/claude_fs_setup.rs index 663e4df65..2e9379c7c 100644 --- a/src/worker/claude_bridge/tests/claude_fs_setup.rs +++ b/src/worker/claude_bridge/tests/claude_fs_setup.rs @@ -1,16 +1,18 @@ //! Tests for Claude filesystem setup utilities. +use crate::test_support::ExpectValid; use rstest::rstest; use super::{build_permission_settings, copy_dir_recursive}; fn parse_allow_list(tools: &[String]) -> Vec { - let json_str = build_permission_settings(tools).expect("permission settings should build"); + let json_str = + build_permission_settings(tools).expect_valid("permission settings should build"); let parsed: serde_json::Value = - serde_json::from_str(&json_str).expect("settings JSON should parse"); + serde_json::from_str(&json_str).expect_valid("settings JSON should parse"); parsed["permissions"]["allow"] .as_array() - .expect("allow list should be an array") + .expect_valid("allow list should be an array") .clone() } @@ -42,56 +44,59 @@ fn test_build_permission_settings( #[test] fn test_copy_dir_recursive() { - let src = tempfile::tempdir().expect("create src tempdir"); - let dst = tempfile::tempdir().expect("create dst tempdir"); + let src = tempfile::tempdir().expect_valid("create src tempdir"); + let dst = tempfile::tempdir().expect_valid("create dst tempdir"); - ambient_fs::write(src.path().join("auth.json"), r#"{"token":"abc"}"#).expect("write auth file"); - ambient_fs::create_dir_all(src.path().join("subdir")).expect("create subdir"); + ambient_fs::write(src.path().join("auth.json"), r#"{"token":"abc"}"#) + .expect_valid("write auth file"); + ambient_fs::create_dir_all(src.path().join("subdir")).expect_valid("create subdir"); ambient_fs::write(src.path().join("subdir").join("nested.txt"), "nested") - .expect("write nested file"); + .expect_valid("write nested file"); - let copied = copy_dir_recursive(src.path(), dst.path()).expect("copy directory tree"); + let copied = copy_dir_recursive(src.path(), dst.path()).expect_valid("copy directory tree"); assert_eq!(copied, 2); assert_eq!( - ambient_fs::read_to_string(dst.path().join("auth.json")).expect("read copied auth file"), + ambient_fs::read_to_string(dst.path().join("auth.json")) + .expect_valid("read copied auth file"), r#"{"token":"abc"}"# ); assert_eq!( ambient_fs::read_to_string(dst.path().join("subdir").join("nested.txt")) - .expect("read copied nested file"), + .expect_valid("read copied nested file"), "nested" ); } #[test] fn test_copy_dir_recursive_empty_source() { - let src = tempfile::tempdir().expect("create src tempdir"); - let dst = tempfile::tempdir().expect("create dst tempdir"); + let src = tempfile::tempdir().expect_valid("create src tempdir"); + let dst = tempfile::tempdir().expect_valid("create dst tempdir"); - let copied = copy_dir_recursive(src.path(), dst.path()).expect("copy empty directory"); + let copied = copy_dir_recursive(src.path(), dst.path()).expect_valid("copy empty directory"); assert_eq!(copied, 0); } #[test] fn test_copy_dir_recursive_skips_nonexistent_source() { - let dst = tempfile::tempdir().expect("create dst tempdir"); - let root = tempfile::tempdir().expect("create source root tempdir"); + let dst = tempfile::tempdir().expect_valid("create dst tempdir"); + let root = tempfile::tempdir().expect_valid("create source root tempdir"); let nonexistent = root.path().join("no_such_path"); - let copied = copy_dir_recursive(&nonexistent, dst.path()).expect("copy should be graceful"); + let copied = + copy_dir_recursive(&nonexistent, dst.path()).expect_valid("copy should be graceful"); assert_eq!(copied, 0); } #[test] fn test_copy_dir_recursive_propagates_destination_errors() { - let src = tempfile::tempdir().expect("create src tempdir"); - let dst = tempfile::tempdir().expect("create dst tempdir"); + let src = tempfile::tempdir().expect_valid("create src tempdir"); + let dst = tempfile::tempdir().expect_valid("create dst tempdir"); - ambient_fs::create_dir_all(src.path().join("subdir")).expect("create source subdir"); + ambient_fs::create_dir_all(src.path().join("subdir")).expect_valid("create source subdir"); ambient_fs::write(src.path().join("subdir").join("nested.txt"), "nested") - .expect("write nested source file"); + .expect_valid("write nested source file"); ambient_fs::write(dst.path().join("subdir"), "not a directory") - .expect("block destination subdir path"); + .expect_valid("block destination subdir path"); let error = copy_dir_recursive(src.path(), dst.path()) .expect_err("destination-side failures should be returned"); diff --git a/src/worker/container/tests/hosted_fidelity.rs b/src/worker/container/tests/hosted_fidelity.rs index 138e4a9e0..8312b6d5f 100644 --- a/src/worker/container/tests/hosted_fidelity.rs +++ b/src/worker/container/tests/hosted_fidelity.rs @@ -4,6 +4,7 @@ //! orchestrator catalogue endpoint and the worker-side proxy without //! field loss or transformation. +use crate::test_support::ExpectValid; use std::sync::Arc; use anyhow::Context as _; @@ -120,7 +121,7 @@ async fn spawn_hosted_catalog_server() -> Result< let server = tokio::spawn(async move { axum::serve(listener, router) .await - .expect("serve hosted fidelity test router") + .expect_valid("serve hosted fidelity test router") }); Ok((format!("http://{addr}"), captured_requests, server)) @@ -168,7 +169,7 @@ async fn hosted_worker_proxy_definition_matches_orchestrator_canonical_definitio .tools .get("complex_orchestrator_wasm_fidelity_fixture") .await - .expect("complex tool proxy should be registered"); + .expect_valid("complex tool proxy should be registered"); let proxy_definition = ToolDefinition { name: proxy_tool.name().to_string(), @@ -224,12 +225,14 @@ async fn hosted_worker_first_llm_request_forwards_wasm_schema_on_first_call( let captured_requests = captured_requests.lock().await; let first_request = captured_requests .first() - .expect("expected one proxied tool-completion request"); + .expect_valid("expected one proxied tool-completion request"); let forwarded_wasm_tool = first_request .tools .iter() .find(|tool| tool.name == "complex_orchestrator_wasm_fidelity_fixture") - .expect("worker should forward the orchestrator-owned WASM tool on the first request"); + .expect_valid( + "worker should forward the orchestrator-owned WASM tool on the first request", + ); assert_eq!( forwarded_wasm_tool, diff --git a/src/worker/container/tests/pre_loop.rs b/src/worker/container/tests/pre_loop.rs index f6301ca83..da856c6d0 100644 --- a/src/worker/container/tests/pre_loop.rs +++ b/src/worker/container/tests/pre_loop.rs @@ -1,5 +1,6 @@ //! Tests for pre-loop failure handling and startup error reporting. +use crate::test_support::ExpectValid; use std::sync::Arc; use axum::http::StatusCode; @@ -33,7 +34,7 @@ fn worker_runtime_new_returns_error_on_config_mismatch( Uuid::nil(), "test".to_string(), ) - .expect("test client should build"), + .expect_valid("test client should build"), ); let result = WorkerRuntime::new( @@ -114,7 +115,7 @@ async fn assert_startup_failure(state: &RuntimeTestState) { let failed_status = statuses .first() .filter(|status| status.state == WorkerState::Failed) - .expect("expected a terminal failed status update"); + .expect_valid("expected a terminal failed status update"); assert_eq!(failed_status.iteration, 0); assert_eq!( failed_status.message.as_deref(), @@ -159,7 +160,7 @@ async fn worker_runtime_reports_failed_status_for_pre_loop_errors( let error = harness .take_runtime() - .expect("harness must contain a runtime") + .expect_valid("harness must contain a runtime") .run() .await .expect_err("expected runtime to fail before the execution loop"); @@ -189,7 +190,7 @@ async fn worker_runtime_emits_failed_status_for_initial_status_rejections() -> a let error = harness .take_runtime() - .expect("harness must contain a runtime") + .expect_valid("harness must contain a runtime") .run() .await; let error = error.expect_err("expected runtime to fail when the initial status is rejected"); @@ -241,10 +242,10 @@ async fn worker_runtime_sanitizes_failure_messages( harness .runtime() - .expect("runtime test harness should contain a runtime") + .expect_valid("runtime test harness should contain a runtime") .report_completion(execution, 7) .await - .expect("report_completion should succeed in test harness"); + .expect_valid("report_completion should succeed in test harness"); let completions = state.completions.lock().await; assert_eq!(completions.len(), 1); @@ -278,7 +279,7 @@ fn worker_runtime_from_env_reads_worker_token() { orchestrator_url: "http://localhost:50051/".to_string(), ..WorkerConfig::default() }) - .expect("from_env should succeed when the worker token is present"); + .expect_valid("from_env should succeed when the worker token is present"); assert_eq!( runtime.client.orchestrator_url(), diff --git a/src/worker/container/tests/remote_tools.rs b/src/worker/container/tests/remote_tools.rs index 8df7bcc57..7eb40312f 100644 --- a/src/worker/container/tests/remote_tools.rs +++ b/src/worker/container/tests/remote_tools.rs @@ -1,5 +1,6 @@ //! Tests for remote tool catalogue registration and toolset instruction merging. +use crate::test_support::ExpectValid; use std::sync::Arc; use anyhow::Context; @@ -126,7 +127,7 @@ where let server = tokio::spawn(async move { axum::serve(listener, router) .await - .expect("serve router in test server") + .expect_valid("serve router in test server") }); Ok((format!("http://{addr}"), server)) } @@ -171,7 +172,7 @@ async fn hosted_worker_remote_tool_catalogue_registers_remote_tools() let remote_tool: Option> = runtime.tools.get("hosted_worker_remote_tool_fixture").await; - let remote_tool = remote_tool.expect("hosted remote tool should be registered"); + let remote_tool = remote_tool.expect_valid("hosted remote tool should be registered"); let expected = expected_remote_tool_definition(); assert_eq!(remote_tool.name(), expected.name); assert_eq!(remote_tool.description(), expected.description); @@ -181,7 +182,7 @@ async fn hosted_worker_remote_tool_catalogue_registers_remote_tools() .tools .get("hosted_worker_remote_wasm_tool_fixture") .await - .expect("hosted remote WASM tool should be registered"); + .expect_valid("hosted remote WASM tool should be registered"); let expected_wasm = expected_remote_wasm_tool_definition(); assert_eq!(remote_wasm_tool.name(), expected_wasm.name); assert_eq!(remote_wasm_tool.description(), expected_wasm.description); @@ -214,7 +215,7 @@ async fn worker_runtime_build_reasoning_context_merges_local_and_remote_tools() .messages .iter() .find(|message| message.content.contains(HOSTED_GUIDANCE_HEADING)) - .expect("expected hosted remote-tool guidance message"); + .expect_valid("expected hosted remote-tool guidance message"); assert!( guidance_message @@ -236,7 +237,7 @@ async fn worker_runtime_build_reasoning_context_merges_local_and_remote_tools() .available_tools .iter() .find(|tool| tool.name == "hosted_worker_remote_tool_fixture") - .expect("reasoning context should expose the hosted remote tool"); + .expect_valid("reasoning context should expose the hosted remote tool"); let expected = expected_remote_tool_definition(); assert_eq!(remote_tool.description, expected.description); assert_eq!(remote_tool.parameters, expected.parameters); @@ -245,7 +246,7 @@ async fn worker_runtime_build_reasoning_context_merges_local_and_remote_tools() .available_tools .iter() .find(|tool| tool.name == "hosted_worker_remote_wasm_tool_fixture") - .expect("reasoning context should expose the hosted remote WASM tool"); + .expect_valid("reasoning context should expose the hosted remote WASM tool"); let expected_wasm = expected_remote_wasm_tool_definition(); assert_eq!(remote_wasm_tool.description, expected_wasm.description); assert_eq!(remote_wasm_tool.parameters, expected_wasm.parameters); diff --git a/src/worker/container/tests/shutdown.rs b/src/worker/container/tests/shutdown.rs index a33a7545a..9c45083b7 100644 --- a/src/worker/container/tests/shutdown.rs +++ b/src/worker/container/tests/shutdown.rs @@ -1,5 +1,6 @@ //! Tests for container delegate shutdown behaviour. +use crate::test_support::ExpectValid; use std::sync::Arc; use anyhow::Result; @@ -43,7 +44,7 @@ async fn spawn_event_server( let handle = tokio::spawn(async move { axum::serve(listener, app) .await - .expect("event test server should run"); + .expect_valid("event test server should run"); }); Ok((format!("http://{addr}"), handle)) } From 4f0dc60bda9412e518aff40e71cf4c9168c23b60 Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 23 Aug 2026 21:45:14 +0200 Subject: [PATCH 3/3] Improve test quality coverage Parameterize equivalent cases, remove tests that only restate their premises, and move the legacy libSQL schema into a named fixture. Keep assertions focused on observable behaviour and make migration input easier to inspect independently. --- src/agent/heartbeat/tests.rs | 112 +++++------------- src/app/tests.rs | 5 - src/cli/doctor/tests.rs | 110 +---------------- src/safety/leak_detector/tests.rs | 28 ++--- src/setup/prompts/tests.rs | 25 ---- tests/db_integration/libsql_wit_defaults.rs | 45 +------ .../libsql/legacy_wasm_wit_schema.sql | 42 +++++++ 7 files changed, 88 insertions(+), 279 deletions(-) create mode 100644 tests/fixtures/libsql/legacy_wasm_wit_schema.sql diff --git a/src/agent/heartbeat/tests.rs b/src/agent/heartbeat/tests.rs index b752d5492..11cc3de84 100644 --- a/src/agent/heartbeat/tests.rs +++ b/src/agent/heartbeat/tests.rs @@ -2,6 +2,8 @@ use std::time::Duration; +use rstest::rstest; + use super::checklist::{is_effectively_empty, strip_html_comments}; use super::*; @@ -29,72 +31,22 @@ fn test_heartbeat_config_builders() { // ==================== strip_html_comments ==================== -#[test] -fn test_strip_html_comments_no_comments() { - assert_eq!(strip_html_comments("hello world"), "hello world"); -} - -#[test] -fn test_strip_html_comments_single() { - assert_eq!( - strip_html_comments("beforeafter"), - "beforeafter" - ); -} - -#[test] -fn test_strip_html_comments_multiple() { - let input = "abc"; - assert_eq!(strip_html_comments(input), "abc"); -} - -#[test] -fn test_strip_html_comments_multiline() { - let input = "# Title\n\nreal content"; - assert_eq!(strip_html_comments(input), "# Title\n\nreal content"); -} - -#[test] -fn test_strip_html_comments_unclosed() { - let input = "beforeafter", "beforeafter")] +#[case::multiple("abc", "abc")] +#[case::multiline( + "# Title\n\nreal content", + "# Title\n\nreal content" +)] +#[case::unclosed("before")); -} - -#[test] -fn test_effectively_empty_empty_checkboxes() { - assert!(is_effectively_empty("# Checklist\n- [ ]\n- [x]")); -} - -#[test] -fn test_effectively_empty_bare_list_markers() { - assert!(is_effectively_empty("-\n*\n-")); -} - -#[test] -fn test_effectively_empty_seeded_template() { - let template = "\ +const SEEDED_HEARTBEAT_TEMPLATE: &str = "\ # Heartbeat Checklist "; - assert!(is_effectively_empty(template)); -} -#[test] -fn test_effectively_empty_real_checklist() { - let content = "\ +#[rstest] +#[case::empty_string("", true)] +#[case::whitespace(" \n\n \n ", true)] +#[case::headers_only("# Title\n## Subtitle\n### Section", true)] +#[case::html_comments_only("", true)] +#[case::empty_checkboxes("# Checklist\n- [ ]\n- [x]", true)] +#[case::bare_list_markers("-\n*\n-", true)] +#[case::seeded_template(SEEDED_HEARTBEAT_TEMPLATE, true)] +#[case::real_checklist( + "\ # Heartbeat Checklist - [ ] Check for unread emails needing a reply -- [ ] Review today's calendar for upcoming meetings"; - assert!(!is_effectively_empty(content)); -} - -#[test] -fn test_effectively_empty_mixed_real_and_headers() { - let content = "# Title\n\nDo something important"; - assert!(!is_effectively_empty(content)); -} - -#[test] -fn test_effectively_empty_comment_plus_real_content() { - let content = "\nActual task here"; - assert!(!is_effectively_empty(content)); + - [ ] Review today's calendar for upcoming meetings", + false +)] +#[case::mixed_real_and_headers("# Title\n\nDo something important", false)] +#[case::comment_plus_real_content("\nActual task here", false)] +fn test_is_effectively_empty(#[case] content: &str, #[case] expected: bool) { + assert_eq!(is_effectively_empty(content), expected); } // ==================== quiet hours ==================== diff --git a/src/app/tests.rs b/src/app/tests.rs index 8489c0922..cc1d78a5e 100644 --- a/src/app/tests.rs +++ b/src/app/tests.rs @@ -21,11 +21,6 @@ use rstest::{fixture, rstest}; #[cfg(feature = "libsql")] use crate::db::libsql::LibSqlBackend; -#[test] -fn runtime_side_effects_new_all_none_does_not_panic() { - let _ = RuntimeSideEffects::new(None, None, None, false); -} - #[tokio::test] async fn runtime_side_effects_start_no_ops_when_nothing_configured() -> anyhow::Result<()> { let se = RuntimeSideEffects::new(None, None, None, false); diff --git a/src/cli/doctor/tests.rs b/src/cli/doctor/tests.rs index 4ede3e83c..5b483c707 100644 --- a/src/cli/doctor/tests.rs +++ b/src/cli/doctor/tests.rs @@ -1,14 +1,10 @@ //! Unit tests for doctor diagnostic checks and result formatting. use super::CheckResult; -use super::core_checks::{ - check_llm_config, check_llm_config_with_context, check_nearai_session, check_settings_file, - check_workspace_dir, check_workspace_search, -}; -use super::external_checks::{check_binary, check_docker_daemon, check_service_installed}; +use super::core_checks::check_llm_config_with_context; +use super::external_checks::check_binary; use super::subsystem_checks::{ - check_embeddings, check_embeddings_with_context, check_gateway_config, check_mcp_config, - check_routines_config, check_routines_config_with_context, check_secrets, check_skills, + check_embeddings_with_context, check_routines_config_with_context, check_secrets, }; use crate::config::EnvContext; use crate::settings::Settings; @@ -32,74 +28,6 @@ fn check_binary_skips_nonexistent() { } } -#[test] -fn check_workspace_dir_does_not_panic() { - let result = check_workspace_dir(); - match result { - CheckResult::Pass(_) | CheckResult::Fail(_) | CheckResult::Skip(_) => {} - } -} - -#[tokio::test] -async fn check_workspace_search_does_not_panic() { - let result = check_workspace_search().await; - match result { - CheckResult::Pass(_) | CheckResult::Fail(_) | CheckResult::Skip(_) => {} - } -} - -#[tokio::test] -async fn check_nearai_session_does_not_panic() { - let result = check_nearai_session().await; - match result { - CheckResult::Pass(_) | CheckResult::Fail(_) | CheckResult::Skip(_) => {} - } -} - -#[test] -fn check_settings_file_handles_missing() { - // Settings::default_path() might or might not exist, but must not panic - let result = check_settings_file(); - match result { - CheckResult::Pass(_) | CheckResult::Fail(_) | CheckResult::Skip(_) => {} - } -} - -#[test] -fn check_llm_config_does_not_panic() { - let settings = Settings::default(); - let result = check_llm_config(&settings); - match result { - CheckResult::Pass(_) | CheckResult::Fail(_) | CheckResult::Skip(_) => {} - } -} - -#[test] -fn check_routines_config_does_not_panic() { - let result = check_routines_config(); - match result { - CheckResult::Pass(_) | CheckResult::Fail(_) | CheckResult::Skip(_) => {} - } -} - -#[test] -fn check_gateway_config_does_not_panic() { - let settings = Settings::default(); - let result = check_gateway_config(&settings); - match result { - CheckResult::Pass(_) | CheckResult::Fail(_) | CheckResult::Skip(_) => {} - } -} - -#[test] -fn check_embeddings_does_not_panic() { - let settings = Settings::default(); - let result = check_embeddings(&settings); - match result { - CheckResult::Pass(_) | CheckResult::Fail(_) | CheckResult::Skip(_) => {} - } -} - #[test] fn check_secrets_none_returns_skip() { let settings = Settings::default(); @@ -117,38 +45,6 @@ fn check_secrets_none_returns_skip() { } } -#[test] -fn check_service_installed_does_not_panic() { - let result = check_service_installed(); - match result { - CheckResult::Pass(_) | CheckResult::Fail(_) | CheckResult::Skip(_) => {} - } -} - -#[tokio::test] -async fn check_docker_daemon_does_not_panic() { - let result = check_docker_daemon().await; - match result { - CheckResult::Pass(_) | CheckResult::Fail(_) | CheckResult::Skip(_) => {} - } -} - -#[tokio::test] -async fn check_mcp_config_does_not_panic() { - let result = check_mcp_config().await; - match result { - CheckResult::Pass(_) | CheckResult::Fail(_) | CheckResult::Skip(_) => {} - } -} - -#[tokio::test] -async fn check_skills_does_not_panic() { - let result = check_skills().await; - match result { - CheckResult::Pass(_) | CheckResult::Fail(_) | CheckResult::Skip(_) => {} - } -} - #[test] fn check_llm_config_shows_nearai_model_for_nearai_backend() { let settings = Settings::default(); diff --git a/src/safety/leak_detector/tests.rs b/src/safety/leak_detector/tests.rs index 1103884e6..c0ac53561 100644 --- a/src/safety/leak_detector/tests.rs +++ b/src/safety/leak_detector/tests.rs @@ -1,5 +1,7 @@ //! Unit tests for secret leak detection and severity grading. +use rstest::rstest; + use crate::safety::leak_detector::{LeakDetector, LeakSeverity}; #[test] @@ -106,12 +108,15 @@ fn test_scan_and_clean_passes_clean() { assert_eq!(result.unwrap(), content); } -#[test] -fn test_mask_secret() { - use crate::safety::leak_detector::mask_secret; - - assert_eq!(mask_secret("short"), "*****"); - assert_eq!(mask_secret("sk-test1234567890abcdef"), "sk-t********cdef"); +#[rstest] +#[case::empty("", "")] +#[case::short("abc", "***")] +#[case::eight_characters("12345678", "********")] +#[case::nine_characters("123456789", "1234*6789")] +#[case::typical_secret("short", "*****")] +#[case::long_secret("sk-test1234567890abcdef", "sk-t********cdef")] +fn test_mask_secret(#[case] secret: &str, #[case] expected: &str) { + assert_eq!(crate::safety::leak_detector::mask_secret(secret), expected); } #[test] @@ -270,17 +275,6 @@ fn test_multiple_different_secret_types() { ); } -#[test] -fn test_mask_secret_short_value() { - use crate::safety::leak_detector::mask_secret; - // Short secrets (<= 8 chars) should be fully masked - assert_eq!(mask_secret("abc"), "***"); - assert_eq!(mask_secret(""), ""); - assert_eq!(mask_secret("12345678"), "********"); - // 9-char string shows first 4 + last 4 with one star in middle - assert_eq!(mask_secret("123456789"), "1234*6789"); -} - #[test] fn test_clean_text_not_flagged() { let detector = LeakDetector::new(); diff --git a/src/setup/prompts/tests.rs b/src/setup/prompts/tests.rs index 24b44c61d..8a1d2fc7d 100644 --- a/src/setup/prompts/tests.rs +++ b/src/setup/prompts/tests.rs @@ -8,31 +8,6 @@ use rstest::rstest; use super::{SecretInputEffect, apply_secret_input_effect, apply_secret_key_event}; -#[test] -fn test_header_length_calculation() { - // Just verify it doesn't panic with various inputs - super::print_header("Test"); - super::print_header("A longer header text"); - super::print_header(""); -} - -#[test] -fn test_step_indicator() { - super::print_step(1, 3, "Test Step"); - super::print_step(3, 3, "Final Step"); -} - -#[test] -fn test_print_functions_do_not_panic() { - super::print_success("operation completed"); - super::print_error("something went wrong"); - super::print_info("here is some information"); - // Also test with empty strings - super::print_success(""); - super::print_error(""); - super::print_info(""); -} - #[rstest] #[case( "", diff --git a/tests/db_integration/libsql_wit_defaults.rs b/tests/db_integration/libsql_wit_defaults.rs index 689102270..6a41ba0a2 100644 --- a/tests/db_integration/libsql_wit_defaults.rs +++ b/tests/db_integration/libsql_wit_defaults.rs @@ -5,50 +5,7 @@ use axinite::db::{Database, libsql::LibSqlBackend}; -const LEGACY_WASM_WIT_SCHEMA: &str = r#" -CREATE TABLE IF NOT EXISTS _migrations ( - version INTEGER PRIMARY KEY, - name TEXT NOT NULL, - applied_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')) -); - -INSERT OR IGNORE INTO _migrations (version, name) -VALUES (9, 'flexible_embedding_dimension'); - -CREATE TABLE IF NOT EXISTS wasm_tools ( - id TEXT PRIMARY KEY, - user_id TEXT NOT NULL, - name TEXT NOT NULL, - version TEXT NOT NULL DEFAULT '1.0.0', - wit_version TEXT NOT NULL DEFAULT '0.1.0', - description TEXT NOT NULL, - wasm_binary BLOB NOT NULL, - binary_hash BLOB NOT NULL, - parameters_schema TEXT NOT NULL, - source_url TEXT, - trust_level TEXT NOT NULL DEFAULT 'user', - status TEXT NOT NULL DEFAULT 'active', - created_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')), - updated_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')), - UNIQUE (user_id, name, version) -); - -CREATE TABLE IF NOT EXISTS wasm_channels ( - id TEXT PRIMARY KEY, - user_id TEXT NOT NULL, - name TEXT NOT NULL, - version TEXT NOT NULL DEFAULT '0.1.0', - wit_version TEXT NOT NULL DEFAULT '0.1.0', - description TEXT NOT NULL DEFAULT '', - wasm_binary BLOB NOT NULL, - binary_hash BLOB NOT NULL, - capabilities_json TEXT NOT NULL DEFAULT '{}', - status TEXT NOT NULL DEFAULT 'active', - created_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')), - updated_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')), - UNIQUE (user_id, name) -); -"#; +const LEGACY_WASM_WIT_SCHEMA: &str = include_str!("../fixtures/libsql/legacy_wasm_wit_schema.sql"); async fn insert_test_wasm_tool(conn: &libsql::Connection) { conn.execute( diff --git a/tests/fixtures/libsql/legacy_wasm_wit_schema.sql b/tests/fixtures/libsql/legacy_wasm_wit_schema.sql new file mode 100644 index 000000000..ef1e9db45 --- /dev/null +++ b/tests/fixtures/libsql/legacy_wasm_wit_schema.sql @@ -0,0 +1,42 @@ +CREATE TABLE IF NOT EXISTS _migrations ( + version INTEGER PRIMARY KEY, + name TEXT NOT NULL, + applied_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')) +); + +INSERT OR IGNORE INTO _migrations (version, name) +VALUES (9, 'flexible_embedding_dimension'); + +CREATE TABLE IF NOT EXISTS wasm_tools ( + id TEXT PRIMARY KEY, + user_id TEXT NOT NULL, + name TEXT NOT NULL, + version TEXT NOT NULL DEFAULT '1.0.0', + wit_version TEXT NOT NULL DEFAULT '0.1.0', + description TEXT NOT NULL, + wasm_binary BLOB NOT NULL, + binary_hash BLOB NOT NULL, + parameters_schema TEXT NOT NULL, + source_url TEXT, + trust_level TEXT NOT NULL DEFAULT 'user', + status TEXT NOT NULL DEFAULT 'active', + created_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')), + updated_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')), + UNIQUE (user_id, name, version) +); + +CREATE TABLE IF NOT EXISTS wasm_channels ( + id TEXT PRIMARY KEY, + user_id TEXT NOT NULL, + name TEXT NOT NULL, + version TEXT NOT NULL DEFAULT '0.1.0', + wit_version TEXT NOT NULL DEFAULT '0.1.0', + description TEXT NOT NULL DEFAULT '', + wasm_binary BLOB NOT NULL, + binary_hash BLOB NOT NULL, + capabilities_json TEXT NOT NULL DEFAULT '{}', + status TEXT NOT NULL DEFAULT 'active', + created_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')), + updated_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')), + UNIQUE (user_id, name) +);