Extract ORM spec models out of describe blocks (issue #169) - #182
Merged
Merged
Conversation
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
spec/support/models/{mongoid,data_mapper}_models.rb are guarded by
if defined?(...) checks that are always false in this repo (no mongoid
or data_mapper gem in any Gemfile), same as the spec files they were
extracted from. Filter them the same way so relocating the guarded
class bodies doesn't dilute measured coverage.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
Review found only a minor documentation line-reference nit, with no approval-blocking issues.
Review effort: Lite
Findings: None
What changed in this PR
Extracts ORM test models from RSpec blocks into auto-loaded support files while preserving behavior and removing the related RuboCop exclusion.
Changes:
- Extracted ActiveRecord, Mongoid, and DataMapper models.
- Updated specs, coverage filters, and RuboCop configuration.
- Added implementation-plan documentation.
| File | Summary |
|---|---|
spec/support/models/mongoid_models.rb |
Extracted Mongoid fixture model. |
spec/support/models/data_mapper_models.rb |
Extracted DataMapper fixture models. |
spec/support/models/active_record_models.rb |
Extracted ActiveRecord fixture models. |
spec/spec_helper.rb |
Added coverage filters for optional ORM models. |
spec/comma/rails/mongoid_spec.rb |
Removed inline Mongoid model definition. |
spec/comma/rails/data_mapper_collection_spec.rb |
Removed inline DataMapper model definition. |
spec/comma/rails/active_record_spec.rb |
Removed inline ActiveRecord model definitions. |
docs/superpowers/plans/2026-09-27-issue-169-orm-model-extraction.md |
Documents the refactor plan. |
.rubocop_todo.yml |
Removed the obsolete exclusion and relocated the string-concatenation exclusion. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
What
Continues issue #169 (spec suite hygiene): moves the ActiveRecord/Mongoid/DataMapper model classes previously defined inside RSpec
describeblocks intospec/support/models/, and reduces.rubocop_todo.ymlaccordingly.Why
spec/comma/rails/active_record_spec.rb,mongoid_spec.rb, anddata_mapper_collection_spec.rbeach defined their ORM model classes inline inside adescribeblock, which required a blanketLint/ConstantDefinitionInBlockexclusion (17 offenses) in.rubocop_todo.yml. Issue #169's acceptance criteria call for reducing at least one such exclusion without changing spec behavior. Since these classes must stay real named constants (ActiveRecord relies on the class name for table/STI inference), the fix is to move them out of the block rather than anonymize them.Changes
if defined?(...)check the spec file already used, and auto-loaded via the existingspec/support/**/*.rbglob inspec_helper.rb:spec/support/models/active_record_models.rb(Picture,Person,Job,PersonFormatter,Animal,Dog,Cat)spec/support/models/mongoid_models.rb(Person)spec/support/models/data_mapper_models.rb(Person,DataMapper.finalize)describeblocks in the 3 spec files down tobefore/itblocks only — no example behavior changed..rubocop_todo.yml: removed theLint/ConstantDefinitionInBlockexclusion entirely (0 offenses now); moved theStyle/StringConcatenationexclusion fromactive_record_spec.rbtoactive_record_models.rb(the offending'Dog-' + namelines moved with the code, not a new offense).spec/spec_helper.rb: added SimpleCovadd_filterentries for the newmongoid_models.rb/data_mapper_models.rb, mirroring the existing filters for their spec files — both are guarded by ORM gems this repo never installs (nomongoid/data_mapperin anyGemfile/gemfiles/*/Appraisals), so without the filter their always-false-guard lines would count as uncovered and dilute the coverage metric.docs/superpowers/plans/2026-09-27-issue-169-orm-model-extraction.mddocumenting the implementation plan.Verified locally:
bundle exec rubocopclean (41 files),bundle exec rspec57/57, andBUNDLE_GEMFILE=gemfiles/active8.1.3.1.gemfile bundle exec rspec68/68 (including the 11-exampleactive_record_spec.rbmatching its pre-change baseline).Note:
mongoid_spec.rb/data_mapper_collection_spec.rb(and their new model files) never actually execute in this repo's CI today — no gemfile installs those gems. That's a pre-existing condition, unchanged by this PR; flagging as a possible follow-up.🤖 Generated with Claude Code