Closed mochetts closed 9 months ago
Edit:
The test passes if I do:
describe "validations" do
it {
should validate_numericality_of(:amount).is_greater_than_or_equal_to(3_000_000).with_message(I18n.t(
"errors.messages.greater_than_or_equal_to",
count: ActiveSupport::NumberHelper.number_to_currency(30000)
))
}
end
I still think the error presented is not friendly/intuitive enough:
NoMethodError:
undefined method `keys' for #<ActiveModel::Errors [#<ActiveModel::Error attribute=amount, type=greater_than_or_equal_to, options={:message=>"must be greater than or equal to $30,000.00", :value=>2999999, :count=>3000000}>]>
Hey @mochetts, thanks for opening this issue. I'll take a look as soon as I have some time!
Hi @mochetts can you try to share a reproducible script of this error? I wasn't able to replicate this error. I know by the log you sent that you are using shoulda-matchers
-4.5.1 and Ruby 3.2.2, but the Rails version might have an impact here.
I tried to reproduce the error on my environment. rails: 7.1.2 ruby: 3.2.2
class MyModel < ApplicationRecord
validates :amount, numericality: {
greater_than_or_equal_to: 3000000,
message: I18n.t(
"errors.messages.greater_than_or_equal_to",
count: ActiveSupport::NumberHelper.number_to_currency(3000000)
)
}
end
require 'rails_helper'
RSpec.describe MyModel, type: :model do
describe "validations" do
it {
should validate_numericality_of(:amount)
.is_greater_than_or_equal_to(3_000_000)
}
end
end
➜ testblog git:(main) ✗ rspec spec/models/my_model_spec.rb
F
Failures:
1) MyModel validations is expected to validate that :amount looks like a number greater than or equal to 3000000
Failure/Error:
should validate_numericality_of(:amount)
.is_greater_than_or_equal_to(3_000_000)
Expected MyModel to validate that :amount looks like a number greater
than or equal to 3000000, but this could not be proved.
After setting :amount to ‹2999999›, the matcher expected the MyModel
to be invalid and to produce the validation error "must be greater
than or equal to 3000000" on :amount. The record was indeed invalid,
but it produced these validation errors instead:
* amount: ["must be greater than or equal to $3,000,000.00"]
# ./spec/models/my_model_spec.rb:6:in `block (3 levels) in <top (required)>'
Finished in 0.02195 seconds (files took 0.68941 seconds to load)
1 example, 1 failure
Failed examples:
rspec ./spec/models/my_model_spec.rb:5 # MyModel validations is expected to validate that :amount looks like a number greater than or equal to 3000000
But if I change my spec file to:
require 'rails_helper'
RSpec.describe MyModel, type: :model do
describe "validations" do
it {
should validate_numericality_of(:amount)
.is_greater_than_or_equal_to(3_000_000)
.with_message('must be greater than or equal to $3,000,000.00')
}
end
end
then tests pass and I think that's expected behavior so I think this is a strong evidence its something rails related. I would love to double check it but I need to know which version of rails @mochetts is using.
Hey! Using 7.1.2
My rails_helper.rb
:
# This file is copied to spec/ when you run 'rails generate rspec:install'
require "spec_helper"
ENV["RAILS_ENV"] ||= "test"
require_relative "../config/environment"
# Prevent database truncation if the environment is production
abort("The Rails environment is running in production mode!") if Rails.env.production?
require "rspec/rails"
require "capybara/rails"
require Rails.root.join("test_helpers/base_test_helper")
require "capybara/cuprite"
require "capybara-screenshot/rspec"
require "active_storage_validations/matchers"
require "pundit/matchers"
require "pundit/rspec"
Dir[Rails.root.join("spec/features/support/**/*.rb")].sort.each { |f| require f }
require "active_support"
require "active_support/testing/time_helpers"
# Add additional requires below this line. Rails is not loaded until this point!
# Requires supporting ruby files with custom matchers and macros, etc, in
# spec/support/ and its subdirectories. Files matching `spec/**/*_spec.rb` are
# run as spec files by default. This means that files in spec/support that end
# in _spec.rb will both be required and run as specs, causing the specs to be
# run twice. It is recommended that you do not name files matching this glob to
# end with _spec.rb. You can configure this pattern with the --pattern
# option on the command line or in ~/.rspec, .rspec or `.rspec-local`.
#
# The following line is provided for convenience purposes. It has the downside
# of increasing the boot-up time by auto-requiring all files in the support
# directory. Alternatively, in the individual `*_spec.rb` files, manually
# require only the support files necessary.
#
# Dir[Rails.root.join('spec', 'support', '**', '*.rb')].sort.each { |f| require f }
# Checks for pending migrations and applies them before tests are run.
# If you are not using ActiveRecord, you can remove these lines.
begin
ActiveRecord::Migration.maintain_test_schema!
rescue ActiveRecord::PendingMigrationError => e
abort e.to_s.strip
end
Shoulda::Matchers.configure do |config|
config.integrate do |with|
with.test_framework :rspec
with.library :rails
end
end
def non_empty_pdf
file = Rack::Test::UploadedFile.new(Rails.root.join("spec/fixtures/files/non_empty.pdf"))
ActiveStorage::Blob.create_and_upload!(
io: file.open,
filename: file.original_filename,
content_type: file.content_type
)
end
# Configure capybara to use cuprite driver for javascript tests
Capybara.register_driver(:cuprite) do |app|
# Unlike the capybara timeout, which should be very short to ensure test fail
# fast, this timeout needs to be long to ensure we give enough time for assets
# to be precompiled.
#
# You can increase this value in `.env.development.local` in slower matchines.
cuprite_timeout = ENV.fetch("CUPRITE_TIMEOUT", 30).to_i
Capybara::Cuprite::Driver.new(
app,
js_errors: true,
browser_options: {
"no-sandbox" => nil,
"disable-dev-shm-usage" => nil,
"disable-site-isolation-trials" => nil
},
timeout: cuprite_timeout,
process_timeout: cuprite_timeout,
inspector: !ENV["CI"],
headless: %w[true 1 yes].include?(ENV["HEADLESS"] || ENV["CI"]),
slowmo: (ENV["SLOWMO"] == "true") ? 0.1 : ENV["SLOWMO"],
logger: CustomCupriteLogger.new
)
end
Capybara.javascript_driver = :cuprite
Capybara.default_driver = :cuprite
def open_file_fixture(file_name)
File.open(file_fixture(file_name))
end
RSpec.configure do |config|
config.include RSpec::Rails::RequestExampleGroup, type: :request
# Remove this line if you're not using ActiveRecord or ActiveRecord fixtures
config.fixture_path = "#{::Rails.root}/spec/fixtures"
config.global_fixtures = :all
# If you're not using ActiveRecord, or you'd prefer not to run each of your
# examples within a transaction, remove the following line or assign false
# instead of true.
config.use_transactional_fixtures = true
# You can uncomment this line to turn off ActiveRecord support entirely.
# config.use_active_record = false
# RSpec Rails can automatically mix in different behaviours to your tests
# based on their file location, for example enabling you to call `get` and
# `post` in specs under `spec/controllers`.
#
# You can disable this behaviour by removing the line below, and instead
# explicitly tag your specs with their type, e.g.:
#
# RSpec.describe UsersController, type: :controller do
# # ...
# end
#
# The different available types are documented in the features, such as in
# https://rspec.info/features/6-0/rspec-rails
# config.infer_spec_type_from_file_location!
# Filter lines from Rails gems in backtraces.
config.filter_rails_from_backtrace!
# arbitrary gems may also be filtered via:
# config.filter_gems_from_backtrace("gem name")
config.include ActiveJob::TestHelper
config.include ActiveStorageValidations::Matchers
config.include DefaultTestHelpers, type: :feature
config.include ActiveSupport::Testing::TimeHelpers
ActiveStorage::Current.url_options = {host: "https://www.example.com"}
end
My spec_helper.rb
require "inertia_rails/rspec"
require "capybara/rspec"
require "capybara_test_helpers/rspec"
# This file was generated by the `rails generate rspec:install` command. Conventionally, all
# specs live under a `spec` directory, which RSpec adds to the `$LOAD_PATH`.
# The generated `.rspec` file contains `--require spec_helper` which will cause
# this file to always be loaded, without a need to explicitly require it in any
# files.
#
# Given that it is always loaded, you are encouraged to keep this file as
# light-weight as possible. Requiring heavyweight dependencies from this file
# will add to the boot time of your test suite on EVERY test run, even for an
# individual file that may not need all of that loaded. Instead, consider making
# a separate helper file that requires the additional dependencies and performs
# the additional setup, and require it from the spec files that actually need
# it.
#
# See https://rubydoc.info/gems/rspec-core/RSpec/Core/Configuration
RSpec.configure do |config|
# rspec-expectations config goes here. You can use an alternate
# assertion/expectation library such as wrong or the stdlib/minitest
# assertions if you prefer.
config.expect_with :rspec do |expectations|
# This option will default to `true` in RSpec 4. It makes the `description`
# and `failure_message` of custom matchers include text for helper methods
# defined using `chain`, e.g.:
# be_bigger_than(2).and_smaller_than(4).description
# # => "be bigger than 2 and smaller than 4"
# ...rather than:
# # => "be bigger than 2"
expectations.include_chain_clauses_in_custom_matcher_descriptions = true
end
# rspec-mocks config goes here. You can use an alternate test double
# library (such as bogus or mocha) by changing the `mock_with` option here.
config.mock_with :rspec do |mocks|
# Prevents you from mocking or stubbing a method that does not exist on
# a real object. This is generally recommended, and will default to
# `true` in RSpec 4.
mocks.verify_partial_doubles = true
end
# This option will default to `:apply_to_host_groups` in RSpec 4 (and will
# have no way to turn it off -- the option exists only for backwards
# compatibility in RSpec 3). It causes shared context metadata to be
# inherited by the metadata hash of host groups and examples, rather than
# triggering implicit auto-inclusion in groups with matching metadata.
config.shared_context_metadata_behavior = :apply_to_host_groups
end
RSpec::Matchers.define :be_the_same_date_as do |expected|
match do |actual|
expect(expected.strftime("%d/%m/%Y")).to eq(actual.strftime("%d/%m/%Y"))
end
end
RSpec::Matchers.define :be_the_same_time_as do |expected|
match do |actual|
expect(expected.strftime("%d/%m/%Y %H:%M:%S")).to eq(actual.strftime("%d/%m/%Y %H:%M:%S"))
end
end
There are 2 errors when the object gets validated.. maybe that's the key. The other error was fixed as stated in https://github.com/thoughtbot/shoulda-matchers/issues/1583. Maybe this got fixed with that as well?
@mochetts can you share your factory_bot file for that model?
I'm using fixtures
wonka_incomplete:
amount: 4000000
avg_draw_amount: 4000000
company: wonka
status: incomplete
Trimmed down version of the spec:
require "rails_helper"
RSpec.describe CreditLineRequest, type: :model do
subject { credit_line_requests(:wonka_incomplete) }
describe "validations" do
it {
should validate_numericality_of(:amount).is_greater_than_or_equal_to(50_00_00).with_message(I18n.t(
"errors.messages.greater_than_or_equal_to",
count: ActiveSupport::NumberHelper.number_to_currency(5000)
))
}
end
end
Trimmed down version of the model:
class CreditLineRequest < ApplicationRecord
belongs_to :company
validates :amount, :avg_draw_amount, presence: true
validates :amount, numericality: {
greater_than_or_equal_to: 5_000_00,
message: I18n.t(
"errors.messages.greater_than_or_equal_to",
count: ActiveSupport::NumberHelper.number_to_currency(5000)
)
}
validates :avg_draw_amount, numericality: {
less_than_or_equal_to: ->(request) { request.amount || 0 },
message: I18n.t(
"errors.messages.less_than_or_equal_to",
count: I18n.t("activerecord.attributes.credit_line_request.amount")
)
}
end
I'm still unable to reproduce the error. :thinking:
@amalrik are you using the latest commit on main? Because there's a chance that this was fixed by https://github.com/thoughtbot/shoulda-matchers/pull/1552
@mochetts this commit https://github.com/thoughtbot/shoulda-matchers/commit/7663821cff15235d1dc8ffc61ca7e8c5e701dc52 seems to be fixing this problem. But the main issue is that your validations are "dependent" on each other, which makes the way shoulda-matchers
uses to check the validations return multiple errors, not a single one.
The code did not support that, and the commit I linked makes this adjusment.
@amalrik are you using the latest commit on main? Because there's a chance that this was fixed by https://github.com/thoughtbot/shoulda-matchers/pull/1552
I'm actually using the last stable release I think it's 5.3.0
@matsales28 @amalrik alright I think I figured it out... I included "shoulda" in the gemfile which pulled "shoulda-matchers" version 4.0.0. So I wasn't pulling all those fixes. The reason of the stale "should-matchers" version is this line: https://github.com/thoughtbot/shoulda/blob/main/shoulda.gemspec#L32
After removing the "shoulda" dependency from the gemfile and instead including the right "shoulda-matchers" dependency:
gem "shoulda-matchers", "~> 6.0"
The error is no longer reproduced.
Thanks for your patience and help! 🙏🏻
@matsales28 @amalrik alright I think I figured it out... I included "shoulda" in the gemfile which pulled "shoulda-matchers" version 4.0.0. So I wasn't pulling all those fixes. The reason of the stale "should-matchers" version is this line: https://github.com/thoughtbot/shoulda/blob/main/shoulda.gemspec#L32
After removing the "shoulda" dependency from the gemfile and instead include the right "shoulda-matchers" dependency:
gem "shoulda-matchers", "~> 6.0"
The error is no longer reproduced.
Thanks for your patience and help! 🙏🏻
Amazing and I'm glad I could help
Description
I'm getting a wrong test failure when adding a custom message to a validator
Model
Test
Error
However, if I remove the custom message the test passes.