Merged
Conversation
Filter out files that shouldn't be included in coverage metrics: - Dummy app scaffolding (application_mailer, job, controller) - Trivial files (version.rb, Rakefile) - Test utility module (testing.rb)
- Enable nocov_token for excluding environment-specific code - Use TARGET_DB in command_name for proper coverage merging when running tests against SQLite, MySQL, and PostgreSQL
Test all workflow error conditions: - RedefiningWorkflowError: when workflow is defined twice - UndefinedWorkflowBlockError: when workflow block is missing - InvalidWorkflowBlockError: when workflow block is invalid - MissingStepsError: when workflow has no steps - UndefinedMethodError: when step uses undefined method - ArgumentMismatchError: when step arity doesn't match - DefinitionMismatchError: when workflow definition changes - UndefinedStepError: when moving to non-existent step
Test Execution model scopes and class methods: - finished scope: returns completed/failed/succeeded executions - outstanding scope: returns non-finished executions - clearable scope: returns finished executions with completed jobs - clear_finished_in_batches: batch deletion with customizable size
Test transactional step plugin validation and behavior: - Validation of transactional option values (true/false/hash) - Transactional behavior with database rollback scenarios - Model-specific transactional wrapping with on: Model
Test GlobalID serialization helpers: - deserialize_global_id with existing/deleted/non-existent records - convert_to_global_id_hash for persisted and new records
Test Entry model scopes and methods: - for_step scope: filters entries by step name - for_action scope: filters entries by action type - ordered scope: orders entries by timestamp - most_recent class method: returns latest entry per step - action? instance method: checks entry action type
Test workflow context persistence and retrieval: - set/get methods for storing and retrieving values - fetch method with default values - []=/[] accessors for hash-like access - Integration with workflow retries (context persistence)
Test Rails Engine configuration and instrumentation: - Engine initialization and configuration - Logger setup and customization - LogSubscriber for workflow instrumentation - Custom serializer registration for workflow arguments
Test plugin API for step customization: - set/get for plugin-local state - current_step accessor - entries_for_action for querying step history - record! for persisting step entries - plugin_action for registering plugin actions - enqueue_job for scheduling continuation jobs - halt_workflow!/repeat_step! for flow control - resolve_method for dynamic method resolution
Test descriptive error messages for debugging: - SucceededStepError includes step name - InvalidMethodError includes method details - DoublePluginCallError includes step context - MissingPluginCallError includes step context
Mark legitimately untestable code paths: - Rails version-specific conditionals in engine.rb - Unknown database adapter fallback in context.rb These code paths depend on environment conditions (Rails version, database adapter) that cannot all be exercised in a single test run.
Update thresholds to reflect improved test coverage: - Overall: 95% line, 80% branch (up from 70%/40%) - Per-file: 80% line, 0% branch (up from 0%/0%) These thresholds ensure coverage doesn't regress as the codebase evolves.
- Replace endless method definitions (Ruby 3.0+) with traditional syntax - Use Module.new instead of module constants inside test methods - Fix array literal spacing per rubocop style
Contributor
There was a problem hiding this comment.
Pull request overview
This PR focuses on significantly increasing test coverage around the core workflow engine, models, plugins, and serializers, while tightening SimpleCov configuration and explicitly marking legitimately untestable branches.
Changes:
- Add comprehensive model and workflow tests covering
ExecutionandEntryscopes, workflow error conditions, context persistence, plugin context behavior, transactional step plugin behavior, and error message formatting. - Expand engine and serializer tests to verify Rails engine wiring, custom serializers, and GlobalID/ActiveJob argument helpers.
- Refine SimpleCov configuration with per-database
command_name, additional filters, a customnocovtoken, and stricter global/per-file coverage thresholds; add:nocov:pragmas around adapter/version-specific branches.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/models/execution_test.rb | Adds tests for AcidicJob::Execution scopes (finished, outstanding, clearable) and clear_finished_in_batches behavior, including batching and custom thresholds. |
| test/models/entry_test.rb | Adds tests for AcidicJob::Entry scopes (for_step, for_action, ordered), most_recent, and the action? instance helper. |
| test/acidic_job/workflow_errors_test.rb | Adds workflow-level tests asserting all custom workflow errors are raised under the correct misuse scenarios and that their messages contain key information. |
| test/acidic_job/plugins/transactional_step_test.rb | Adds validation tests for TransactionalStep.validate and integration tests for transactional behavior of steps, including the { on: Model } configuration. |
| test/acidic_job/plugin_context_test.rb | Exercises the PluginContext API (set, get, current_step, entries_for_action, record!, plugin_action, enqueue_job, halt_workflow!, repeat_step!, resolve_method) through small plugin-instrumented jobs. |
| test/acidic_job/errors_test.rb | Extends existing error-constructor tests with assertions on error message content for SucceededStepError, InvalidMethodError, DoublePluginCallError, and MissingPluginCallError. |
| test/acidic_job/engine_test.rb | Verifies the engine type and name, config access (config.acidic_job), logger configurability, registered log subscriber methods, and that custom serializers can round-trip their supported types. |
| test/acidic_job/context_test.rb | Adds unit tests for AcidicJob::Context#set, get, fetch, []/[]=, plus an integration test validating that context values persist correctly across job retries. |
| test/acidic_job/arguments_test.rb | Tests AcidicJob::Arguments.deserialize_global_id behavior on existing/missing records and convert_to_global_id_hash for persisted and new records. |
| lib/acidic_job/engine.rb | Wraps configuration and version-dependent serializer initialization blocks in # :nocov: markers without changing runtime behavior. |
| lib/acidic_job/context.rb | Wraps the adapter fallback else branch for Value.upsert_all in # :nocov: markers to exclude rare adapter-specific fallback logic from coverage. |
| .simplecov | Configures a nocov token, per-database command_name, additional filters for dummy/scaffolding and trivial files, and raises minimum global and per-file coverage thresholds. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Remove test for unsupported transactional: { on: Model } feature
- Remove unused variable assignments in entry and execution tests
- Replace fragile sleep(0.01) with travel_to in entry_test.rb for deterministic timing - Add setup block to plugin_context_test.rb to reset cattr_accessor values between tests - Rename MissingMethodJob to PluginContextMissingMethodJob to avoid class name conflict - Improve metaprogramming robustness in execution_test.rb with proper begin/ensure structure
- plugin_context_test.rb: Replace cattr_accessor with thread-local TestCapture module for cleaner test isolation and parallel test safety - context_test.rb: Add detailed explanatory comment for the retry persistence test - errors_test.rb: Extract create_test_plugin and create_named_plugin_instance helpers to reduce duplication - execution_test.rb: Simplify batch_size test to verify functional behavior instead of monkeypatching internal methods - .simplecov: Use regex paths for scaffolding filters to be more precise
- Use explicit timestamps in entry_test.rb to avoid flaky ordering - Add rationale comment for SimpleCov coverage thresholds - Add teardown cleanup for thread-local storage in plugin_context_test.rb
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.
Improve test coverage to 97% line / 86% branch
This PR significantly improves test coverage by adding comprehensive tests for previously uncovered code paths. The focus was on encoding core user-facing contracts rather than chasing coverage metrics—these tests document expected behavior and will catch regressions during refactoring.
Coverage improvement
New test files
workflow_errors_test.rb- Tests all 8 workflow error conditions (RedefiningWorkflowError, UndefinedWorkflowBlockError, etc.)execution_test.rb- Tests Execution model scopes (finished,outstanding,clearable) andclear_finished_in_batchestransactional_step_test.rb- Tests TransactionalStep plugin validation and rollback behaviorarguments_test.rb- Tests GlobalID serialization/deserialization helpersentry_test.rb- Tests Entry model scopes andmost_recentmethodcontext_test.rb- Tests workflow context persistence across retriesengine_test.rb- Tests Rails Engine configuration, logger, and custom serializersplugin_context_test.rb- Tests the full PluginContext API for step customizationerrors_test.rb- Extended with tests for error message formattingSimpleCov configuration improvements
command_namebased onTARGET_DBfor proper coverage merging across database runsNotes
:nocov:pragmas for legitimately untestable code (Rails version checks, unknown DB adapter fallback)