Skip to content

Conversation

@DOsinga
Copy link
Collaborator

@DOsinga DOsinga commented Jul 28, 2025

Have scenario tests per provider. We are still missing quite some coverage, but it is a start!

@DOsinga DOsinga requested a review from jamadeo July 28, 2025 12:13
CallToolResult, Implementation, InitializeResult, ListPromptsResult, ListResourcesResult,
ListToolsResult, ReadResourceResult, ServerCapabilities, ToolsCapability,
};
use mcp_core::{Tool, ToolError};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you import Tool from rmcp::model instead?

Sorry, I should have removed the old definition when I finished the migration between those types here https://github.com/block/goose/pull/3617/files

I'll do that as a small PR, but good to start importing the new types

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, it's been a moving target - when I merged in master I had to update all the references again :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would you mind if we do that in a follow up? it gives me some weird type errors

Copy link
Collaborator

@jamadeo jamadeo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very cool. So if I understand correctly, in replay mode, this will replay the provider-specific recordings, but through a mock provider? And then in recording mode we can actually test the providers end-to-end.

return Ok(());
}

let configs_to_test: Vec<_> = if let Some(to_skip) = providers_to_skip {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can get an iterator from an Option<Vec> with option.iter().flatt() -- useful for this kind of thing

}
}

// Rest of the function remains the same...
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see you, llm!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ha!


#[tokio::test]
async fn test_weather_tool() -> Result<()> {
// Google tells me it only knows about the weather in the US, so we skip it.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose we could ask it about the weather somewhere it knows about

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could, but I am guessing it will still refuse to call the tool? I'm just trying to get this in first and then we can make it work for more and more things

@DOsinga DOsinga merged commit 73a274d into main Jul 28, 2025
8 checks passed
@DOsinga DOsinga deleted the provider-scenario-tests branch July 28, 2025 18:04
michaelneale added a commit that referenced this pull request Jul 29, 2025
* main:
  blog: streamlining detection development w/ recipes (#3689)
  fix: have option for cli providers to use their configured or default model  (#3683)
  docs: new blog post and corrections to an old one on goosehints (#3657)
  Resolve sub recipe path relative to the parent recipe path (#3642)
  Speed up recipe loading from deeplinks and various fixes (#3662)
  fix cmd + , not opening settings (#3694)
  Add warning when JSON env parsing fails. (#3696)
  chore: refactor session naming into provider (#3678)
  feat (ui): File picker for scheduling recipes default to recipe dir (#3611)
  fix: address issue with streamable http interactions via mcp (#3693)
  Provider scenario tests (#3688)
  Fix conversations before they hit the LLM (#3660)
  cli: add detailed instruction for WSL users (#3496)
  feat: recipe runs will now prompt for missing extension secrets (#3668)
  fix: pricing integration tests -> trying more runs for cache and retries (#3546)
michaelneale added a commit that referenced this pull request Jul 29, 2025
* main:
  fix: Ensures final output tool is available when using vector tool search (#3701)
  chore: adding in some new models token limits (#3685)
  blog: streamlining detection development w/ recipes (#3689)
  fix: have option for cli providers to use their configured or default model  (#3683)
  docs: new blog post and corrections to an old one on goosehints (#3657)
  Resolve sub recipe path relative to the parent recipe path (#3642)
  Speed up recipe loading from deeplinks and various fixes (#3662)
  fix cmd + , not opening settings (#3694)
  Add warning when JSON env parsing fails. (#3696)
  chore: refactor session naming into provider (#3678)
  feat (ui): File picker for scheduling recipes default to recipe dir (#3611)
  fix: address issue with streamable http interactions via mcp (#3693)
  Provider scenario tests (#3688)
atarantino pushed a commit to atarantino/goose that referenced this pull request Aug 5, 2025
Co-authored-by: Douwe Osinga <[email protected]>
Signed-off-by: Adam Tarantino <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants