Switch test framework from Minitest to RSpec + FactoryBot#13
Conversation
- Replace minitest gem with rspec-rails and factory_bot_rails - Convert all 175 tests from test/ to spec/ (models, requests, services, helpers) - Convert YAML fixtures to FactoryBot factories with proper association chains - Factories derive org from parent objects to keep data consistent - Add sign_in_as helper and ActiveSupport::Testing::TimeHelpers to rails_helper - Update AGENTS.md to document new testing conventions - Remove old test/ directory Amp-Thread-ID: https://ampcode.com/threads/T-019c8c51-0ef2-723d-bab8-a95e7ecbf4e0 Co-authored-by: Amp <amp@ampcode.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8acc8fe625
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| FactoryBot.define do | ||
| factory :edit_lease do | ||
| plan | ||
| organization |
There was a problem hiding this comment.
This factory currently builds plan and organization independently, so create(:edit_lease) can generate cross-tenant records where edit_lease.organization_id does not match plan.organization_id; that state cannot be produced by EditLease.acquire! and can cause specs to pass or fail against impossible data. AGENTS.md explicitly requires factories to derive associations from parent objects, so this should be tied to the plan (organization { plan.organization }).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 75fbce0 — organization now derives from plan.organization.
|
|
||
| trait :resolved do | ||
| status { "resolved" } | ||
| association :resolved_by_user, factory: :user |
There was a problem hiding this comment.
In the :resolved trait, resolved_by_user is created with a standalone :user factory, so it can belong to a different organization than the comment_thread; this again violates the AGENTS.md factory rule about deriving associations from parent objects and introduces invalid multi-tenant test fixtures that can hide authorization/scoping issues when this trait is used. The trait should scope resolved_by_user to plan.organization (or organization).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 75fbce0 — resolved_by_user now scoped to plan.organization.
- edit_leases: derive organization from plan.organization - comment_threads: scope resolved_by_user to plan.organization in :resolved trait Amp-Thread-ID: https://ampcode.com/threads/T-019c9060-1543-77b2-af4d-2757ba2a6ee1 Co-authored-by: Amp <amp@ampcode.com>
1 participant
Summary
Migrates the entire test suite from Minitest + fixtures to RSpec + FactoryBot.
Changes
minitest ~> 5.25, addedrspec-railsandfactory_bot_railsto test groupsign_in_ashelper, TimeHelpersKey conventions
ApplicationRecord#assign_uuid— factories never setidorganization { plan.organization })sign_in_as(user)helper for request specsbundle exec rspecVerification
All 175 examples pass: