test: fix recipe and audio tests to avoid side effects#6231
test: fix recipe and audio tests to avoid side effects#6231codefromthecrypt merged 2 commits intoblock:mainfrom
Conversation
- recipe.rs: Use injectable opener pattern to prevent tests from launching Goose Desktop. Added handle_open_with() with injectable opener and output writer. Created run_handle_open() test helper. Added failure case tests for opener errors and invalid recipes. - audio.rs: Use EnvVarGuard for tokio-safe env var manipulation in test_transcribe_endpoint_requires_auth to avoid keychain access. Signed-off-by: Adrian Cole <adrian@tetrate.io>
3d82f17 to
c006038
Compare
|
ran |
| let params = vec!["name=Alice".to_string(), "role=developer".to_string()]; | ||
| let result = handle_open(&recipe_path, ¶ms); | ||
| // The result may be Ok or Err depending on whether the system can open the URL | ||
| // In a test environment, it will likely fail to open, but that's fine |
There was a problem hiding this comment.
this sounds like an LLM wanting to write a test but not knowing how to
There was a problem hiding this comment.
Pull request overview
This PR fixes test side effects by preventing external system interactions during test execution. The main changes involve migrating from a custom EnvVarGuard to the env-lock crate for thread-safe environment variable management, and introducing dependency injection for the recipe opener to avoid launching Goose Desktop during tests.
- Replaced custom
EnvVarGuardwithenv-lockcrate for tokio-safe environment variable management - Added injectable opener pattern to
recipe.rsto mock external app launches in tests - Set
OPENAI_API_KEYbeforeAppState::new()in audio tests to prevent keychain prompts
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/goose/Cargo.toml | Added env-lock as dev dependency for test environment management |
| crates/goose-server/Cargo.toml | Added env-lock as dev dependency for test environment management |
| crates/goose/src/providers/factory.rs | Migrated from custom EnvVarGuard to env-lock::lock_env for thread-safe env var handling in tests |
| crates/goose-server/src/routes/audio.rs | Added env var setup before AppState::new() to prevent keychain access during tests |
| crates/goose-cli/src/commands/recipe.rs | Extracted handle_open_with to enable dependency injection of opener function, added tests for error paths |
| Cargo.lock | Updated lock file with env-lock v1.0.1 dependency |
1550098 to
da35fdc
Compare
| if let Ok(limit) = limit_str.parse::<usize>() { | ||
| worker_config = worker_config.with_context_limit(Some(limit)); | ||
| } | ||
| if let Ok(limit) = global_config.get_param::<usize>("GOOSE_WORKER_CONTEXT_LIMIT") { |
There was a problem hiding this comment.
unrelated bug but fixed while here. We didn't actually read anything in our previous tests so they were not really helpful before
da35fdc to
db34962
Compare
| let result = create("openai", ModelConfig::new_or_fail("gpt-4o-mini")).await; | ||
|
|
||
| match result { | ||
| Ok(_) => {} |
There was a problem hiding this comment.
rewrote all the assertions here as they did nothing
* main: (155 commits) remove Tool Selection Strategy preview (#6250) fix(cli): correct bash syntax in terminal integration functions (#6181) fix : opening a session to view it modifies session history order in desktop (#6156) test: fix recipe and audio tests to avoid side effects (#6231) chore: Update gemini versions in test_providers.sh (#6246) feat: option to stream json - jsonl really (#6228) feat: add mcp app renderer (#6095) docs: update skills extension to support .agents/skills directories (#6199) Add YouTube short to Chrome DevTools MCP tutorial (#6244) docs: Caveats for privacy information in logs documentation (#6218) move goose issue solver to opus (#6233) feat: improved UX for tool calls via execute_code (#6205) Blog: Code Mode Doesn't Replace MCP (#6227) fix: prevent keychain requests during cargo test (#6219) test: fix test_max_turns_limit slow execution and wrong message type (#6221) Skills vs MCP blog (#6220) Add blog post: Does Your AI Agent Need a Plan? (#6209) fix(ui): enable MCP UI to send a prompt message when an element is clicked (#6207) docs: param option for recipe deeplink/open (#6206) docs: edit in place or fork session (#6203) ...
Summary
Fix tests that cause side effects during
cargo test:open::that(). Added injectable opener pattern withhandle_open_with()so tests can mock the opener and verify URLs without launching external apps. Added failure case tests.EnvVarGuardfor tokio-safe env var manipulation to setOPENAI_API_KEYbeforeAppState::new().Type of Change
AI Assistance
Testing
cargo testpasses with no keychain popups or Goose Desktop launchestest_handle_open_opener_failsandtest_handle_open_invalid_recipeto verify error handling pathsRelated Issues
Completes #6219