Shutdown Tests and Config Consolidation #57
Merged
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
- Move YAML config types (File, Logging, QueueStorage, etc.) to daemon/config.go - Move ApplyConfigFile() and helper functions to daemon package - Update cmd/querator/server.go to use daemon.File and daemon.ApplyConfigFile - Move config tests to daemon/config_test.go - Delete config/ package entirely
- Add service/shutdown_test.go with comprehensive shutdown tests - Test ProduceThenShutdown: verify items persist across restart (BadgerDB, PostgreSQL) - Test LeaseThenShutdown: verify leased items persist in storage after restart - Test PartialCompleteThenShutdown: verify completed items removed, uncompleted remain - Test ShutdownDuringActiveRequests: verify graceful handling of in-flight requests - Skip persistence tests for InMemory backend (not persistent) - Use StorageItemsList for persistence verification (avoids lifecycle loop timing)
thrawn01
commented
Dec 12, 2025
- Remove InMemory backend from shutdown tests since all tests require persistence
- Add deferred cleanup to ensure daemon is shut down even if test fails early - Prevents port 2319 from being left in use causing subsequent tests to fail
- Use fresh context with timeout for deferred cleanup instead of canceled context - Prevents goroutine leaks when shutdown is called with already-canceled context
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.
Purpose
This PR consolidates the
config/package intodaemon/and adds comprehensive shutdown behavior tests. The config package functionality is more appropriately located in daemon since it directly populatesdaemon.Configand handles application startup concerns.Implementation
Config Package Consolidation: Moved all YAML config types (
File,Logging,QueueStorage,PartitionStorage,Queue,Partition) andApplyConfigFile()function fromconfig/todaemon/config.go. Updatedcmd/querator/server.goto import onlydaemoninstead ofconfig. Deleted theconfig/package entirely.Shutdown Behavior Tests: Added
service/shutdown_test.gowith four test scenarios:ProduceThenShutdown: Verifies items produced before shutdown survive restart (BadgerDB, PostgreSQL only)LeaseThenShutdown: Verifies leased items persist in storage after restartPartialCompleteThenShutdown: Verifies completed items are removed while uncompleted items remainShutdownDuringActiveRequests: Verifies graceful handling of in-flight requests during shutdown (all backends)Test Design: Tests use
StorageItemsListfor persistence verification to avoid lifecycle loop timing dependencies. Persistence tests are correctly skipped for InMemory backend since it doesn't persist data across restarts.