Two SDK unit tests from the built-ins rework pass without exercising what they claim to check.
-
Vacuous resolver test. test_a_bare_tool_name_string_is_ignored_too (sdks/python/oss/tests/pytest/unit/agents/tools/test_resolver.py, line 243) calls ToolResolver().resolve(coerce_tool_configs(["read"])). coerce_tool_configs returns a ToolConfigParseResult pydantic model, not a sequence of tool configs, and iterating a pydantic model yields (field, value) tuples. So resolve() never sees the coerced BuiltinToolConfig and returns an empty spec list for the wrong reason: the test is green without exercising the coercion path. It should pass coerce_tool_configs(["read"]).tool_configs.
-
Substring JSON check. test_no_json_example_writes_a_builtin_tool_entry (sdks/python/oss/tests/pytest/unit/agents/test_agenta_builtins_reference_files.py, line 168) checks each JSON example block in the builtins reference with '"type": "builtin"' not in block (line 174). Any whitespace variant such as "type":"builtin" or "type" : "builtin" slips through. The test should parse each block as JSON and recursively reject any object with type == "builtin".
Follow-up from review comments on #5651:
Two SDK unit tests from the built-ins rework pass without exercising what they claim to check.
Vacuous resolver test.
test_a_bare_tool_name_string_is_ignored_too(sdks/python/oss/tests/pytest/unit/agents/tools/test_resolver.py, line 243) callsToolResolver().resolve(coerce_tool_configs(["read"])).coerce_tool_configsreturns aToolConfigParseResultpydantic model, not a sequence of tool configs, and iterating a pydantic model yields(field, value)tuples. Soresolve()never sees the coercedBuiltinToolConfigand returns an empty spec list for the wrong reason: the test is green without exercising the coercion path. It should passcoerce_tool_configs(["read"]).tool_configs.Substring JSON check.
test_no_json_example_writes_a_builtin_tool_entry(sdks/python/oss/tests/pytest/unit/agents/test_agenta_builtins_reference_files.py, line 168) checks each JSON example block in the builtins reference with'"type": "builtin"' not in block(line 174). Any whitespace variant such as"type":"builtin"or"type" : "builtin"slips through. The test should parse each block as JSON and recursively reject any object withtype == "builtin".Follow-up from review comments on #5651: