Incremental spec suite hygiene - #180
Merged
Merged
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The changes are test/support-only refactors that appear behavior-preserving and the helper loading mechanism is already in place via spec/spec_helper.rb.
Pull request overview
This PR incrementally improves RSpec suite hygiene by introducing small support helpers that avoid global class-constant pollution and reduce brittle hand-written CSV expectation strings, while keeping the existing example structure and intent intact.
Changes:
- Added
define_comma_classhelper to build anonymous classes (optionally subclassing a provided base) for specs. - Added
expect_csvhelper to generate expected CSV viaCSV.generatefrom headers/rows and compare to actual output. - Migrated several specs to use these helpers and narrowed
.rubocop_todo.ymlby removing theLint/ConstantDefinitionInBlockexclusion forspec/comma/comma_spec.rb.
File summaries
| File | Description |
|---|---|
| spec/support/comma_class_helper.rb | Adds define_comma_class to avoid defining global constants in examples. |
| spec/support/csv_expectation_helper.rb | Adds expect_csv to generate expected CSV strings consistently via CSV.generate. |
| spec/controllers/users_controller_spec.rb | Replaces several heredoc CSV expectations with expect_csv where the shape matches. |
| spec/comma/comma_spec.rb | Migrates inline class constants and hand-rolled CSV strings to the new helpers. |
| spec/comma/data_extractor_spec.rb | Uses define_comma_class instead of ad-hoc Class.new(Struct...) definitions. |
| spec/comma/header_extractor_spec.rb | Uses define_comma_class instead of ad-hoc Class.new(Struct...) definitions. |
| .rubocop_todo.yml | Removes the Lint/ConstantDefinitionInBlock exclusion for spec/comma/comma_spec.rb. |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Reduces spec pollution and copy-paste per #169: introduces define_comma_class and expect_csv, migrates comma_spec.rb's inline class constants and hand-rolled CSV strings to use them, and applies the same helpers in data_extractor_spec.rb, header_extractor_spec.rb, and users_controller_spec.rb. Drops comma_spec.rb from the Lint/ConstantDefinitionInBlock exclude list now that it defines no class constants. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Class.new(base, &block) already handles a nil block gracefully, so drop the redundant class_eval wrapper that raised ArgumentError when called without one. Also convert the remaining Class.new(Struct.new) call sites in comma_spec.rb and the last straightforward CSV heredoc in users_controller_spec.rb to the new helpers for consistency. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
eitoball
force-pushed
the
eitoball/incremental-spec-suite-hygiene
branch
from
September 5, 2026 12:12
61000e5 to
7f08010
Compare
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
Adds two RSpec support helpers and uses them to remove inline class-constant pollution and hand-rolled CSV strings from the spec suite (#169).
Why
The spec suite documents DSL behavior well but accumulates maintenance cost:
describe/beforeblocks define real global constants (class Foo,class MySuperClass, ...), which pollutes the namespace and forces aLint/ConstantDefinitionInBlockRuboCop exclusion, and several specs hand-roll long expected CSV strings that are easy to typo and hard to diff.Changes
spec/support/comma_class_helper.rb—define_comma_class(base = Object, &block)builds an anonymous class (optionally subclassing a given base, e.g. aStruct) instead of defining a named constant.spec/support/csv_expectation_helper.rb—expect_csv(output, headers:, rows:, **csv_options)builds the expected string viaCSV.generateand asserts equality, instead of hand-writing comma/quote-escaped strings.spec/comma/comma_spec.rb: replace all inlineclass Foo/Bar/MySuperClass/ChildClassComma/ChildClassNoComma/ReopenedSuperClass/ReopenedChildNoCommaconstants (including the STI and class-reopening tests) withdefine_comma_class, and replace the hand-rolled CSV expectation strings withexpect_csv. Same examples, same expectations — only how the class/expected-string is built changes.define_comma_classinspec/comma/data_extractor_spec.rbandspec/comma/header_extractor_spec.rb, replacing the existingClass.new(Struct.new(...))pattern.expect_csvinspec/controllers/users_controller_spec.rbfor the CSV-comparison cases the helper cleanly covers (default, style, header-toggle-on, force_quotes); left the BOM, header-toggle-off, and combined col_sep/row_sep cases as plain heredocs since they don't fit the helper's shape..rubocop_todo.yml: dropspec/comma/comma_spec.rbfrom theLint/ConstantDefinitionInBlockexclude list now that it defines no class constants (verified via a fresh--auto-gen-configrun — the remaining 3 files there are Rails/Mongoid/DataMapper model specs, out of scope).Verified locally: full non-Rails RSpec suite (57 examples) passes,
bundle exec rubocop(37 files) is clean.spec/controllers/users_controller_spec.rbonly runs underif defined?(Rails); a local Rails gemfile couldn't be built in this sandbox (unrelated nokogiri native-build issue), so those specific changes were verified by hand/ruby -cand rely on CI's Appraisal matrix for execution.