test: add unit tests for agent context and tool registry#171
Merged
asdek merged 2 commits intovxcontrol:feature/next_releasefrom Mar 2, 2026
Merged
Conversation
Add unit tests for two core infrastructure modules: context.go - Agent context management: - GetAgentContext on empty context returns false - PutAgentContext first call sets both parent and current - PutAgentContext chaining promotes current to parent - Triple chaining verifies parent tracks previous current - Context isolation between independent context trees registry.go - Tool type mapping and registry: - ToolType.String() for all 8 enum values - GetToolType maps known tool names to correct types - GetToolType returns NoneToolType for unknown tools - Completeness check ensures all mapped tools resolve - GetToolTypeMapping returns a safe copy (mutation test) - GetToolsByType filters correctly and handles empty types - Consistency between toolsTypeMapping and registryDefinitions Signed-off-by: mason5052 <ehehwnwjs5052@gmail.com>
There was a problem hiding this comment.
Pull request overview
Adds unit test coverage for core tooling infrastructure in backend/pkg/tools, focusing on agent context propagation and tool registry/type mapping correctness.
Changes:
- Added
context_test.gocoveringPutAgentContext/GetAgentContextbehavior across empty/first/chained contexts and isolation. - Added
registry_test.gocoveringToolType.String(),GetToolType, reverse mapping viaGetToolsByType, and registry/type-mapping consistency and copy semantics.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| backend/pkg/tools/context_test.go | New tests validating agent context storage/retrieval and chaining semantics. |
| backend/pkg/tools/registry_test.go | New tests validating tool type stringification, tool-name→type mapping, reverse mapping, and registry/mapping consistency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+91
to
+110
| // TestRegistryDefinitionsReturnsCopy verifies that GetRegistryDefinitions returns | ||
| // a copy that can be mutated without affecting the original registry. | ||
| func TestRegistryDefinitionsReturnsCopy(t *testing.T) { | ||
| defs1 := GetRegistryDefinitions() | ||
| originalLen := len(defs1) | ||
|
|
||
| defs1["test_sentinel"] = defs1[TerminalToolName] | ||
|
|
||
| defs2 := GetRegistryDefinitions() | ||
| if len(defs2) != originalLen { | ||
| t.Errorf("mutation leaked: original len = %d, new len = %d", originalLen, len(defs2)) | ||
| } | ||
| if _, ok := defs2["test_sentinel"]; ok { | ||
| t.Error("mutation leaked: test_sentinel found in fresh copy") | ||
| } | ||
| } | ||
|
|
||
| // TestToolTypeMappingReturnsCopy verifies that GetToolTypeMapping returns a copy. | ||
| func TestToolTypeMappingReturnsCopy(t *testing.T) { | ||
| m1 := GetToolTypeMapping() |
There was a problem hiding this comment.
Two tests here don't call t.Parallel(), but the PR description says the new tests use t.Parallel(). Either update the description or add t.Parallel() to these tests for consistency with the rest of the file/package.
Comment on lines
+112
to
+118
|
|
||
| m1["test_sentinel"] = NoneToolType | ||
|
|
||
| m2 := GetToolTypeMapping() | ||
| if len(m2) != originalLen { | ||
| t.Errorf("mutation leaked: original len = %d, new len = %d", originalLen, len(m2)) | ||
| } |
There was a problem hiding this comment.
TestToolTypeMappingReturnsCopy only checks the length of the fresh mapping. If the sentinel key ever already exists, this can become a false negative. Consider first asserting the sentinel key is absent, and also asserting it remains absent in the second mapping (similar to TestRegistryDefinitionsReturnsCopy).
Suggested change
| m1["test_sentinel"] = NoneToolType | |
| m2 := GetToolTypeMapping() | |
| if len(m2) != originalLen { | |
| t.Errorf("mutation leaked: original len = %d, new len = %d", originalLen, len(m2)) | |
| } | |
| if _, ok := m1["test_sentinel"]; ok { | |
| t.Fatalf("precondition failed: test_sentinel already present in tool type mapping") | |
| } | |
| m1["test_sentinel"] = NoneToolType | |
| m2 := GetToolTypeMapping() | |
| if len(m2) != originalLen { | |
| t.Errorf("mutation leaked: original len = %d, new len = %d", originalLen, len(m2)) | |
| } | |
| if _, ok := m2["test_sentinel"]; ok { | |
| t.Error("mutation leaked: test_sentinel found in fresh tool type mapping copy") | |
| } |
Comment on lines
+94
to
+106
| defs1 := GetRegistryDefinitions() | ||
| originalLen := len(defs1) | ||
|
|
||
| defs1["test_sentinel"] = defs1[TerminalToolName] | ||
|
|
||
| defs2 := GetRegistryDefinitions() | ||
| if len(defs2) != originalLen { | ||
| t.Errorf("mutation leaked: original len = %d, new len = %d", originalLen, len(defs2)) | ||
| } | ||
| if _, ok := defs2["test_sentinel"]; ok { | ||
| t.Error("mutation leaked: test_sentinel found in fresh copy") | ||
| } | ||
| } |
There was a problem hiding this comment.
These copy-safety tests intentionally mutate the returned maps. Adding a cleanup (delete the sentinel key) will prevent cascading failures if the test fails partway through or if the implementation is accidentally changed to return the underlying global map.
…eness Fix style issues identified during self-review: - Add t.Parallel() to TestRegistryDefinitionsReturnsCopy - Add t.Parallel() to TestToolTypeMappingReturnsCopy - Add reverse direction check in TestRegistryDefinitionsCompleteness to verify tools in registryDefinitions also exist in toolsTypeMapping Signed-off-by: mason5052 <ehehwnwjs5052@gmail.com>
Contributor
|
tnx! |
30 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Add unit tests for two core infrastructure modules in
backend/pkg/tools/:context.go- Agent context management (PutAgentContext/GetAgentContext)registry.go- Tool type mapping, registry definitions, and type filteringType of Change
Areas Affected
backend/pkg/tools/context_test.go(new file)backend/pkg/tools/registry_test.go(new file)Testing
All tests are self-contained, table-driven where appropriate, and use
t.Parallel().context_test.go (5 test functions):
TestGetAgentContextEmpty- empty context returns falseTestPutAgentContextFirst- first call sets both parent and current to same agentTestPutAgentContextChaining- second call promotes current to parent, sets new currentTestPutAgentContextTriple- triple chaining verifies parent tracks previous currentTestPutAgentContextIsolation- independent context trees do not interfereregistry_test.go (6 test functions):
TestToolTypeString- all 8 ToolType enum values map to correct string representationsTestGetToolType- known tool names map to correct types, unknown returns NoneToolTypeTestGetToolTypeMappingCompleteness- all mapped tools resolve to non-None typeTestGetToolTypeMappingCopySafety- GetToolTypeMapping returns a copy (mutation does not affect original)TestGetToolsByType- filters tools correctly by type, empty result for unused typesTestToolTypeMappingRegistryConsistency- every tool in type mapping exists in registry definitionsSecurity Considerations
No security impact - test-only changes.
Checklist