Update unit test coverage for tools + secret packages #85
Conversation
saucow
changed the title
saucow
changed the title
CONTRIBUTING.md
Outdated
| # Run integration tests (requires Docker plugin to be installed) | ||
| make integration | ||
|
|
||
| # Run integration tests with console output |
There was a problem hiding this comment.
question: Did this mean to say "unit tests"?
There was a problem hiding this comment.
Ah nice catch, updated it to remove the integration command to avoid confusion
| ### Unit Test Coverage | ||
|
|
||
| ```bash | ||
| # Generate HTML coverage report for ALL packages in one view |
cmd/docker-mcp/tools/enable_test.go
Outdated
There was a problem hiding this comment.
suggestion: I would recommend keeping all tools.go tests in a single tools_test.go file. In my experience that's golang convention for unit tests, but also I think it would make browsing go source files difficult with lots of test files bloating the filesystem. wdyt?
There was a problem hiding this comment.
@cmrigney - good idea, we don't have many tests to begin with so let's stick with that pattern and can re-visit it as needed. Updated to keep the tests in tools_test.go
Add coverage for package secret Linter issues Consolidate tools tests into tools test file Clean-up test structure update contributing
saucow
force-pushed
the
unit-test-coverage-updated
branch
from
9791244 to
06c9f7d
Compare
|
Note - Merging this as commit is verified and approved - seeing lingering CodeQL issue for older commits that have been squashed. |
What I did
Test coverage needs to be improved.
CONTRIBUTING.mdupdated with Testing section - which contains commands for quickly understanding current coverage and snippets for analyzing integration test outputIncreased coverage for
toolspackage to:58.9%from36.0%(Note this is only for unit, integration coverage is higher)Added tests for
secretpackage.set.gocoverage is now60%Note: This PR focused on adding simple tests for input criteria - subsequent test changes will tackle mocking providers where it makes sense to increase additional test coverage
Related issue
36.0%of statements. See screenshot below for tools package file coverage breakdown:0.0%(not mandatory) A picture of a cute animal, if possible in relation to what you did
🕸️