Coverage and cleanup #172
Merged
Coverage and cleanup #172
Conversation
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
Handler Refactoring: - Move GetStatistics, AddFunction, RemoveFunction, AddExecutor, AddAttribute, SetOutput from controller command queue to direct database calls in handlers - Each handler now defines its own minimal Server interface - Remove unused controller adapters from server_adapter.go - Update tests to use direct DB calls Database Connection Pool: - Add configurable connection pool settings via environment variables: COLONIES_DB_MAX_OPEN_CONNS, COLONIES_DB_MAX_IDLE_CONNS, COLONIES_DB_CONN_MAX_LIFETIME, COLONIES_DB_CONN_MAX_IDLE_TIME - Add STATE-only indexes for PROCESSES and PROCESSGRAPHS tables - Add connection pool settings to docker-compose.yml Cleanup TODOs: - Add TODO comments to ResetDatabase, RemoveAllProcesses, RemoveAllProcessGraphs documenting orphaned channel cleanup issues in clustered deployments Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Bug fixes: - Fix progress bar leak in Download when Quiet=true - pw was created but never stopped, causing goroutine leak - Fix DownloadSnapshot not checking error from GetFileByID before using result, which could cause nil pointer dereference Tests: - Add error path tests for CalcChecksumsAndCompare - Add tests for printSyncPlan with empty/nil sync plans - Add edge case tests for RemoveFilesNotInDir - Add invalid checksum handling tests - Add CalcEtag error handling tests 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The old-style reconciliation embedded Old/New blueprints in the process FunctionSpec. This is replaced by the cron-based approach that passes blueprintName via KwArgs and fetches blueprints from the server. Changes: - Remove Reconciliation field from FunctionSpec struct - Remove RECONCILIATION column from database schema - Simplify process handler to only use FuncName="reconcile" approach - Remove obsolete tests for embedded reconciliation The Reconciliation type itself is kept as it's still used for computing diffs when updating blueprints (to detect spec changes and bump generation). Note: Existing databases need recreation or manual column drop. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add TestGetVersion for HandleVersion (was at 0% coverage) - Add TestGetStatisticsUnauthorized for auth failure path - Add TestGetClusterInfoUnauthorized for auth failure path Improves server handler coverage from 56.5% to 65.9%. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Blueprint Architecture: - Remove BlueprintHandler struct and Handler field from Blueprint - Blueprint instances no longer override handler config from definition - BlueprintDefinition.Spec.Handler is now the single source of truth - Server uses definition handler for executor type and function name FunctionSpec Cleanup: - Remove unused Blueprint field from FunctionSpec struct - Database still writes empty string to BLUEPRINT column for compatibility - Skip deserialization of blueprint data from database Handler Refactoring: - Rename defaultExecutorType to executorType (not overridable) - Set NodeName to executorType-functionName format for workflow DAGs - Use BlueprintDefinition.Spec.Handler.FunctionName instead of hardcoding CLI Updates: - Update blueprint commands to get handler from BlueprintDefinition - Remove handler display from blueprint table (not in Blueprint anymore) Tests: - Remove TestBlueprintInFunctionSpec (field removed) - Update handler tests to verify new NodeName format - Remove tests for blueprint handler override behavior Cleanup: - Delete TODO.md (completed tasks) - Delete CHANGES.md (outdated) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove duplicate StaleExecutorDuration declaration in root.go - Update server_adapter.go to match simplified Controller interface: - Remove GetGenerator, ResolveGenerator, GetGenerators from generatorControllerAdapter - Remove GetCron, GetCronByName, GetCrons from cronControllerAdapter - Add missing CronDB() method to ServerAdapter - Remove ProcessGraphDB-related code from server handlers unit tests: - MockProcessGraphDB no longer needed (workflow stats removed from global stats) - Remove four workflow counting error tests These changes align the codebase after rebasing onto main, which moved some handler operations from controller methods to direct DB calls. Generated with Claude Code Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
johankristianss
force-pushed
the
coverage_and_cleanup
branch
from
0bad910 to
96e7145
Compare
- Add TestCloseWithContext to verify successful close with context - Add TestCloseWithContextCancelled to verify cancelled context behavior - Fix colonies_controller_test.go: change := to = for already declared err - Fix colonies_controller_worker_test.go: set staleExecutorDuration field before calling cleanupStaleExecutors() (method no longer takes parameter) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The ProcessNotFound tests were expecting StatusInternalServerError (500) but the handler correctly returns StatusNotFound (404) when process is nil. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- TestHandleRemoveFunction_FunctionNotFound: expect 404 (NotFound) not 400 - TestHandleRemoveFunction_GetFunctionError: expect 400 (BadRequest) not 403 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Process handler tests: - Fix TestHandleGetProcess_NotFound: expect 404 (NotFound) not 500 - Fix TestHandleRemoveProcess_Success: set process state to PENDING (handler rejects removing RUNNING processes) - Fix TestHandleRemoveAllProcesses: use processDB mock instead of controller (handler now uses ProcessDB directly) - Add removeAllProcessesErr field to MockProcessDB ProcessGraph handler tests: - Add MockProcessGraphDB with all required interface methods - Add ProcessGraphDB() method to MockServer Makefile: - Enable fs package tests (uncommented) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Rename TestHandleRemoveProcessGraph_ControllerError to _DBError - Rename TestHandleRemoveAllProcessGraphs_ControllerError to _DBError - Use processGraphDB.removeErr and removeAllErr instead of controller - Update RemoveAllWaitingProcessGraphsByColonyName mock to return error The handlers now use ProcessGraphDB directly instead of Controller. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Update test to expect "reconcile_worker" (from BlueprintDefinition) instead of hardcoded "reconcile". The handler uses the FunctionName configured in the BlueprintDefinition. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add the following packages to the test target: - pkg/server/handlers/blueprint - pkg/server/handlers/channel - pkg/server/handlers/location - pkg/client - pkg/client/backends - pkg/server/registry - pkg/validate - pkg/channel These were being tested by coverage (go list ./pkg/...) but not by make test, causing test failures to go undetected until coverage ran. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
johankristianss
force-pushed
the
coverage_and_cleanup
branch
from
ecb48e9 to
a4baf82
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #172 +/- ##
==========================================
- Coverage 70.49% 70.19% -0.31%
==========================================
Files 239 239
Lines 19327 19475 +148
==========================================
+ Hits 13625 13670 +45
+ Misses 3844 3550 -294
- Partials 1858 2255 +397 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
No description provided.